2

I'm working on an app where I need to call one of two data methods based on the generic type of the calling class. For example, if T is of type Foo, I'll call data.GetFoo():

private static List<T> GetObjectList(DateTime mostRecentProcessedReadTime)
{
    using (MesReportingDal data = new MesReportingDal())
    {
        return data.GetFoo(mostRecentProcessedReadTime);  // Notice GetFoo()
    }
}

And if T is of type Bar, I'll call data.GetBar():

private static List<T> GetObjectList(DateTime mostRecentProcessedReadTime)
{
    using (MesReportingDal data = new MesReportingDal())
    {
        return data.GetBar(mostRecentProcessedReadTime);  // Notice GetBar()
    }
}

Before now, I only needed one DAL method because all types were retrieved the same way. I now need to call one of two methods, depending on the type of T.

I'm trying to avoid something like this:

private static List<T> GetObjectList(DateTime mostRecentProcessedReadTime)
{
    using (MesReportingDal data = new MesReportingDal())
    {
        if (T is Foo) { return data.GetFoo(mostRecentProcessedReadTime); }
        if (T is Bar) { return data.GetBar(mostRecentProcessedReadTime); }
    }
}

This violates OCP. Is there an elegant way to handle this, so I can get rid of my if statement?

Edit - This is what the types look like

public partial class Foo1 : IDataEntity { }
public partial class Foo2 : IDataEntity { }
public partial class Bar1 : IDataEntity { }
public partial class Bar2 : IDataEntity { }

These Foos and Bars are the DBML items used with Linq-to-SQL.

Bob Horn
  • 33,387
  • 34
  • 113
  • 219
  • do you have a control over 'T' classes - can they be changed, do you have base interface/class etc. – NSGaga-mostly-inactive Apr 20 '12 at 13:54
  • If you don't wish to change Foo*, Bar* and it has to be GetFoo, GetBar - not Get() as ataddeini suggested (if I understood right?). How do you intend to 'call' the GetObjectList()? are you defining e.g. Foo, Bar before the call (so you have e.g. a typed variable) - or you want to call this in some highly generic way (using T from another generic method)? Easiest is what ataddeini said, if you can. – NSGaga-mostly-inactive Apr 20 '12 at 14:19
  • The use of `static` worries me already. Can you explain how you think this violates OCP? Without context, it's hard to tell. –  Apr 20 '12 at 14:47
  • @NSGaga I might be able to do what ataddeini said. I haven't had time just yet. I'll check it out soon. – Bob Horn Apr 20 '12 at 15:08
  • @jeyoung It violates OCP because we have to remember the if statement and add to it for each new type. – Bob Horn Apr 20 '12 at 15:08
  • If you have a `` anywhere in your code, you'll end up violating OCP anyway because somewhere down the line you will need to figure what `T` is and write code specifically for the type. It seems like you have a very narrow interpretation of OCP. It doesn't mean that your class needs to be generic to satisfy OCP; you can have very specific classes (e.g. one bound to a specif type) as long as they are extensible. So, go ahead, lose the `static` and make specific methods for `Foo` and `Bar` (unless they are related by type ancestry in some way, in which case generics is the solution). –  Apr 20 '12 at 15:25
  • I never said my class needs to be generic to satisfy OCP. The class was generic before this problem existed. And I may indeed lose the static. I'm not arguing against any of this. I just haven't had time to try the suggestions yet. – Bob Horn Apr 20 '12 at 15:29

1 Answers1

3

I would change GetFoo and GetBar to just be Get, and make MesReportingDal a generic too.

So I think you would end up with something like this:

private static List<T> GetObjectList(DateTime mostRecentProcessedReadTime)
{
    using (var data = new MesReportingDal<T>())
    {
        return data.Get(mostRecentProcessedReadTime);        
    }
}

Incidentally, having the using statement also requires that the MesReportingDal implements IDisposable, otherwise you'll get the following compile error:

'MesReportingDal': type used in a using statement must be implicitly convertible to 'System.IDisposable'

UPDATE

So after thinking about this some more and reading your feedback, one option you have is to extract a repository interface and push the creation back to a factory method. This will allow you to maintain the single data.Get(...) call, but with different implementations based on T

public interface IRepository<T> : IDisposable
{
    IList<T> Get(DateTime mostRecentRead);
}

public class FooRepo : IRepository<Foo>
{
    public IList<Foo> Get(DateTime mostRecentRead)
    {
        // Foo Implementation
    }
}

public class BarRepo : IRepository<Bar>
{
    public IList<Bar> Get(DateTime mostRecentRead)
    {
        // Bar Implemenation
    }
}

Your factory could then look something like this

public class RepositoryFactory
{
    public static IRepository<T> CreateRepository<T>()
    {
        IRepository<T> repo = null;
        Type forType = typeof(T);

        if (forType == typeof(Foo))
        {
            repo = new FooRepo() as IRepository<T>;
        }
        else if (forType == typeof(Bar))
        {
            repo = new BarRepo() as IRepository<T>;
        }

        return repo;
    }
}

And this would allow you to keep your initial code block clean

private static IList<T> GetObjectList(DateTime mostRecentProcessedReadTime)
{
    using (var data = RepositoryFactory.CreateRepository<T>())
    {
        return data.Get(mostRecentProcessedReadTime);
    }
}

Hope that helps.

ataddeini
  • 4,931
  • 26
  • 34
  • I don't know that this will work, and it may be because I wasn't very clear. The data.Get() method has different implementations based on the type. The algorithm within Get() is different. – Bob Horn Apr 20 '12 at 15:52
  • @BobHorn Right, ok. Thinking about this more, I keep coming back to a repository interface and a factory. I'll try to update shortly with an example. – ataddeini Apr 21 '12 at 12:21
  • Thanks. While this pushes the if logic somewhere else, it indeed keeps the consuming code clean. I was hoping to avoid *if* logic altogether, but this solution at least keeps it within a factory. I think my only other option is reflection, but this can work for now. – Bob Horn Apr 21 '12 at 17:51
  • FYI, This line ends up returning null: repo = new FooRepo() as IRepository; I don't need it solved, but I at least wanted to let you know. – Bob Horn Apr 23 '12 at 13:02