In this topic here I have aksed for help to sort out an error while creating a generic class whose method can return a different collection given a type. I received the answer and working solution but I have a feeling the problem is not sorted in proper way. That is why I came here for a review.
The reason for having a generic class is is that I use DI and pass either DbContext
to collect data from a database or another Object to "collect" data from collections. The DI is done in the main Program
. The interface is generic to handle DbContext
and then fetching particular collection given a type: DbContext.Set<>()
. The FetchDataFromCollection
is also generic, so is FetchDataFromDatabase
because this is how DbCOntext
works.
Is there any other way to sort it out and for instance avoid if ... else blocks? I was thinking to make another class ModelBase
, inherits from it into concrete implementations, which would return a specific collection and inject such class into FetchFromCollection
but then I would have to make this decition by myself which class to inject to get proper collection in FetchFromCollection
. What I would have is a method which would decide to return a particular collection given a type.
I don't know if it's possible. Below are the three program components.
IFetchData
:
public interface IFetchData<TEntity>
{
IEnumerable<TEntity> GetItems();
}
Then I have created these two classes:
FetchFromDatabase
:
public class FetchFromDatabase<TEntity> : IFetchData<TEntity>
where TEntity : class
{
private readonly MainDBContextBase context;
public FetchFromDatabase(MainDBContextBase context)
{
if (context == null)
throw new ArgumentNullException("DB context");
this.context = context;
}
public IEnumerable<TEntity> GetItems()
{
return context.Set<TEntity>();
}
}
FetchFromCollection
:
public class FetchFromCollection<TEntity> : IFetchData<TEntity>
where TEntity : class
{
// ModelBase<TEntity> models;
private readonly InitializeComponents components;
private ModelModules modules;
private ModelSpecializations specializations;
private ModelTeachers teachers;
private ModelStudents students;
public FetchFromCollection(InitializeComponents components)
{
if (components == null)
throw new ArgumentNullException("Context");
this.components = components;
}
public IEnumerable<TEntity> GetItems()
{
if (typeof(TEntity) == typeof(Module))
{
if (modules == null)
modules = new ModelModules(components);
return (IEnumerable<TEntity>)modules.Set();
}
if (typeof(TEntity) == typeof(Specialization))
{
if (specializations == null)
specializations = new ModelSpecializations(components);
return (IEnumerable<TEntity>)specializations.Set();
}
if (typeof(TEntity) == typeof(Module))
{
if (teachers == null)
teachers = new ModelTeachers(components);
return (IEnumerable<TEntity>)teachers.Set();
}
else
{
if (students == null)
students = new ModelStudents(components);
return (IEnumerable<TEntity>)students.Set();
}
}
}
2 Answers 2
It seems like you could just mirror the way FetchFromDatabase
is implemented. First thing you will need is a something like a DbContext, but for collections. This new class should be capable of returning an IEnumerable<TEntity>
for any TEntity
defined -- just like DbContext
.
public class CollectionContext
{
private Dictionary<Type, object> _cache;
public CollectionContext()
{
_cache = new Dictionary<Type, object>();
}
public void AddCollection<TEntity>(IEnumerable<TEntity> collection)
{
_cache.Add(typeof(TEntity), collection);
}
public IEnumerable<TEntity> Set<TEntity>()
{
return (IEnumerable<TEntity>)_cache[typeof(TEntity)];
}
}
Now FetchFromCollection
can be implemented exactly like FetchFromDatabase
, with exception of the type used in the constructor.
public class FetchFromCollection<TEntity> : IFetchData<TEntity>
where TEntity : class
{
private readonly CollectionContext context;
public FetchFromCollection(CollectionContext context)
{
if (context == null)
throw new ArgumentNullException("Collection context");
this.context = context;
}
public IEnumerable<TEntity> GetItems()
{
return context.Set<TEntity>();
}
}
This is how you'd use it.
// Load up all your collections
var context = new CollectionContext();
context.AddCollection<Module>(new ModelModules(components).Set());
context.AddCollection<Specialization>(new ModelSpecializations(components).Set());
context.AddCollection<Teacher>(new ModelTeachers(components).Set());
// Example usage
IFetchData<Module> modules = new FetchFromCollection<Module>(context);
I think you can further improve on your design by getting rid of the generic type parameter on FetchFromDatabase
and FetchFromCollection
. Each of these classes is capable of pulling any TEntity
out of their contexts'. By making this change you'd only need to have one instance of FetchFromDatabase
and FetchFromCollection
for your entire application, instead of one instance of each for every TEntity
. You can just move the generic type parameter down to GetItems()
. This same change would need to be made to IFetchData
. Here is an example for FetchFromCollection
, the same change can be made to FetchFromDatabase
:
public class FetchFromCollection : IFetchData
{
private readonly CollectionContext context;
public FetchFromCollection(CollectionContext context)
{
if (context == null)
throw new ArgumentNullException("Collection context");
this.context = context;
}
public IEnumerable<TEntity> GetItems<TEntity>()
{
return context.Set<TEntity>();
}
}
-
\$\begingroup\$ Thanks man. I really appreciate your effort. It did not occur to me that I could use
Dictionary<Type, Object>
. Somehow, I did not imagine that theType
could be a value and consequently a key for a dictionary. Also, thanks for pointing out the issue about having a generic class :) It's really valueable answer to me! I would give you more than one vote if I could. \$\endgroup\$Celdor– Celdor2015年10月04日 15:27:41 +00:00Commented Oct 4, 2015 at 15:27 -
\$\begingroup\$ Hello again. I did try to improve the function
FetchFromCollection
to avoid the generic type. I found problem here. First I removed it from class, so compiler complained as Interface is generic. I removed a generic type from the interface but then a return type,IEnumerable<TEntity>
, has to be still generic. So I cannot fulfil with this improvement :/ \$\endgroup\$Celdor– Celdor2015年10月05日 13:21:50 +00:00Commented Oct 5, 2015 at 13:21 -
\$\begingroup\$ It should work if you make the method generic. It should be GetItems<TEntity>() like in my example. Also make sure you change
IFetchData
interface. \$\endgroup\$Mike Hixson– Mike Hixson2015年10月05日 17:04:17 +00:00Commented Oct 5, 2015 at 17:04 -
\$\begingroup\$ @MikeHixon I am probably missing something very trivial here. Currently, the interface is generic because the method
IEnumerable<TEntity> GetItems()
returns collection of items of the typeTEntity
. if I remove<TEentity>
from interface, I don't know what type shouldGetItems()
return then. \$\endgroup\$Celdor– Celdor2015年10月06日 06:52:14 +00:00Commented Oct 6, 2015 at 6:52 -
\$\begingroup\$ You can have a method that is generic without the whole class being generic. Try using the FetchFromCollection class just as I have it above, but without the interface. Checkout how it works, then add the interface back when you understand it. \$\endgroup\$Mike Hixson– Mike Hixson2015年10月06日 07:46:07 +00:00Commented Oct 6, 2015 at 7:46
I ended up doing the same thing as Mike, but went one step further:
Removed classes
Getting the items into CollectionContext
can be simplified, there is no need for the ModuleXXX
-classes. The different items can be added directly:
context = new CollectionContext();
context.Append(new[] {new Module()});
context.Append(new[] {new Specialization()});
context.Append(new[] {new Teacher()});
context.Append(new[] {new Student()});
The usage is identical to Mike's, but I added a static GetItem
method to force the code to use the IFetchData
interface:
var items = GetItems(new FetchFromCollection<Module>(context));
static IEnumerable<T> GetItems<T>(IFetchData<T> fetcher)
{
return fetcher.GetItems();
}
By the way, I suspect you are not into Test Driven Development? I suggest you give it a try. Just by writing the expected client side code using FetchFromCollection
would address some of your questions.
CollectionContext
public class CollectionContext
{
readonly List<object> collection = new List<object>();
public void Append(IEnumerable<object> modules)
{
collection.AddRange(modules);
}
public IEnumerable<T> Set<T>()
{
return collection.OfType<T>();
}
}
Tests
[TestFixture]
public class Given_some_known_collections
{
CollectionContext context;
[SetUp]
public void Setup()
{
context = new CollectionContext();
context.Append(new[] {new Module()});
context.Append(new[] {new Specialization()});
context.Append(new[] {new Teacher()});
context.Append(new[] {new Student()});
}
[Test]
public void Get_modules()
{
var items = GetItems(new FetchFromCollection<Module>(context));
Assert.That(items.Count(), Is.EqualTo(1));
}
[Test]
public void Get_specializations()
{
var items = GetItems(new FetchFromCollection<Specialization>(context));
Assert.That(items.Count(), Is.EqualTo(1));
}
[Test]
public void Get_teachers()
{
var items = GetItems(new FetchFromCollection<Teacher>(context));
Assert.That(items.Count(), Is.EqualTo(1));
}
[Test]
public void Get_students()
{
var items = GetItems(new FetchFromCollection<Student>(context));
Assert.That(items.Count(), Is.EqualTo(1));
}
static IEnumerable<T> GetItems<T>(IFetchData<T> fetcher)
{
return fetcher.GetItems();
}
}
-
\$\begingroup\$ I am new to TDD and it's not the first time I've been suggested to use tests. Perhaps the time to to start with it is now :) \$\endgroup\$Celdor– Celdor2015年10月05日 07:54:23 +00:00Commented Oct 5, 2015 at 7:54
DbContext
has a method calledDbSet<> Set<>()
. By providing a type, we query for records in a table. The object somehow has its intelligence to in decision which table to query. I tried to do this for collection in memory. \$\endgroup\$Fetch...
is to encapsulate the methodIEnumerable<TEntity> GetItems()
because this is how I belive the Dependency Injection work; we define a field of an interface type and later inject objects which use common methods. \$\endgroup\$