Gotta Love Refactoring
Posted by Davy Brion on April 19th, 2009
Behold the following two classes:
public class FutureCriteriaBatch
{
private readonly List<ICriteria> criterias = new List<ICriteria>();
private readonly IList<System.Type> resultCollectionGenericType = new List<System.Type>();
private int index;
private IList results;
private readonly ISession session;
public FutureCriteriaBatch(ISession session)
{
this.session = session;
}
public IList Results
{
get
{
if (results == null)
{
var multiCriteria = session.CreateMultiCriteria();
for (int i = 0; i < criterias.Count; i++)
{
multiCriteria.Add(resultCollectionGenericType[i], criterias[i]);
}
results = multiCriteria.List();
((SessionImpl)session).FutureCriteriaBatch = null;
}
return results;
}
}
public void Add<T>(ICriteria criteria)
{
criterias.Add(criteria);
resultCollectionGenericType.Add(typeof(T));
index = criterias.Count - 1;
}
public void Add(ICriteria criteria)
{
Add<object>(criteria);
}
public IFutureValue<T> GetFutureValue<T>()
{
int currentIndex = index;
return new FutureValue<T>(() => (IList<T>)Results[currentIndex]);
}
public IEnumerable<T> GetEnumerator<T>()
{
int currentIndex = index;
return new DelayedEnumerator<T>(() => (IList<T>)Results[currentIndex]);
}
}
and
public class FutureQueryBatch
{
private readonly List<IQuery> queries = new List<IQuery>();
private readonly IList<System.Type> resultCollectionGenericType = new List<System.Type>();
private int index;
private IList results;
private readonly ISession session;
public FutureQueryBatch(ISession session)
{
this.session = session;
}
public IList Results
{
get
{
if (results == null)
{
var multiQuery = session.CreateMultiQuery();
for (int i = 0; i < queries.Count; i++)
{
multiQuery.Add(resultCollectionGenericType[i], queries[i]);
}
results = multiQuery.List();
((SessionImpl)session).FutureQueryBatch = null;
}
return results;
}
}
public void Add<T>(IQuery query)
{
queries.Add(query);
resultCollectionGenericType.Add(typeof(T));
index = queries.Count - 1;
}
public void Add(IQuery query)
{
Add<object>(query);
}
public IFutureValue<T> GetFutureValue<T>()
{
int currentIndex = index;
return new FutureValue<T>(() => (IList<T>)Results[currentIndex]);
}
public IEnumerable<T> GetEnumerator<T>()
{
int currentIndex = index;
return new DelayedEnumerator<T>(() => (IList<T>)Results[currentIndex]);
}
}
They are almost exactly the same, which is obviously a problem. The root cause of this situation is that there is no common interface between IQuery and ICriteria, and there also isn’t a common interface between IMultiQuery and IMultiCriteria. Introducing those interfaces would make a lot of things easier, but can’t be done right now. Still, that shouldn’t prevent us from striving to avoid duplicate code.
This is actually pretty easy to do… here’s the new FutureBatch class:
public abstract class FutureBatch<TQueryApproach, TMultiApproach>
{
private readonly List<TQueryApproach> queries = new List<TQueryApproach>();
private readonly IList<System.Type> resultTypes = new List<System.Type>();
private int index;
private IList results;
protected readonly SessionImpl session;
protected FutureBatch(SessionImpl session)
{
this.session = session;
}
public IList Results
{
get
{
if (results == null)
{
GetResults();
}
return results;
}
}
public void Add<TResult>(TQueryApproach query)
{
queries.Add(query);
resultTypes.Add(typeof(TResult));
index = queries.Count - 1;
}
public void Add(TQueryApproach query)
{
Add<object>(query);
}
public IFutureValue<TResult> GetFutureValue<TResult>()
{
int currentIndex = index;
return new FutureValue<TResult>(() => GetCurrentResult<TResult>(currentIndex));
}
public IEnumerable<TResult> GetEnumerator<TResult>()
{
int currentIndex = index;
return new DelayedEnumerator<TResult>(() => GetCurrentResult<TResult>(currentIndex));
}
private void GetResults()
{
var multiApproach = CreateMultiApproach();
for (int i = 0; i < queries.Count; i++)
{
AddTo(multiApproach, queries[i], resultTypes[i]);
}
results = GetResultsFrom(multiApproach);
ClearCurrentFutureBatch();
}
private IList<TResult> GetCurrentResult<TResult>(int currentIndex)
{
return (IList<TResult>)Results[currentIndex];
}
protected abstract TMultiApproach CreateMultiApproach();
protected abstract void AddTo(TMultiApproach multiApproach, TQueryApproach query, System.Type resultType);
protected abstract IList GetResultsFrom(TMultiApproach multiApproach);
protected abstract void ClearCurrentFutureBatch();
}
And now the original 2 classes look like this:
public class FutureCriteriaBatch : FutureBatch<ICriteria, IMultiCriteria>
{
public FutureCriteriaBatch(SessionImpl session) : base(session) {}
protected override IMultiCriteria CreateMultiApproach()
{
return session.CreateMultiCriteria();
}
protected override void AddTo(IMultiCriteria multiApproach, ICriteria query, System.Type resultType)
{
multiApproach.Add(resultType, query);
}
protected override IList GetResultsFrom(IMultiCriteria multiApproach)
{
return multiApproach.List();
}
protected override void ClearCurrentFutureBatch()
{
session.FutureCriteriaBatch = null;
}
}
and
public class FutureQueryBatch : FutureBatch<IQuery, IMultiQuery>
{
public FutureQueryBatch(SessionImpl session) : base(session) {}
protected override IMultiQuery CreateMultiApproach()
{
return session.CreateMultiQuery();
}
protected override void AddTo(IMultiQuery multiApproach, IQuery query, System.Type resultType)
{
multiApproach.Add(resultType, query);
}
protected override IList GetResultsFrom(IMultiQuery multiApproach)
{
return multiApproach.List();
}
protected override void ClearCurrentFutureBatch()
{
session.FutureQueryBatch = null;
}
}
They are still very similar, but at least it’s a big improvement over the original situation.
April 22nd, 2009 at 11:30 am
Thanks for sharing, I din’t notice this before.