1
\$\begingroup\$

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();
        }
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Would using a generic method instead of a generic class help? I'm unclear on what it is that you're really trying to accomplish. \$\endgroup\$ – RubberDuck Oct 3 '15 at 15:22
  • \$\begingroup\$ DbContext has a method called DbSet<> 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\$ – Celdor Oct 3 '15 at 16:13
  • \$\begingroup\$ I don't know if a generic method would work. I inject one of these objects to another object which later present queried data. Technically the class Fetch... is to encapsulate the method IEnumerable<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\$ – Celdor Oct 3 '15 at 16:19
3
\$\begingroup\$

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>();
    }
}
\$\endgroup\$
  • \$\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 the Type 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 Oct 4 '15 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 Oct 5 '15 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 Oct 5 '15 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 type TEntity. if I remove <TEentity> from interface, I don't know what type should GetItems() return then. \$\endgroup\$ – Celdor Oct 6 '15 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 Oct 6 '15 at 7:46
1
\$\begingroup\$

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();
    }
}
\$\endgroup\$
  • \$\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 Oct 5 '15 at 7:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.