HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Generic data interface

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
interfacegenericdata

Problem

I currently have the following working code. I am looking for some suggestion with best practices and perhaps a better way to accomplish my goal.

Goal: - in short - Have a generic data interface as a single data access point to different data sources.

Concern: My current code is working fine but I have to call GetRepository() in each method inside my test class to access the desire instance of the data source repository.

Code Snippet:

public class GenericRepository : IGenericRepository
{
   public T GetRepository() 
    {
        string typeName = typeof(T).ToString();
        if (string.IsNullOrEmpty(typeName))
            return default(T);

        Type repoType = Type.GetType(typeName);
        object repoInstance = Activator.CreateInstance(repoType);
        return (T)(object)repoInstance;
    }
    // more code... 
}


Usage:

public class MyTestClass
{
    IGenericRepository repo = new GenericRepository();

    private void DoThis()
    {
       IFileRepository repoA = repo.GetRepository();
       // now I can call repository A interface methods like this repoA.SomeMethod()
    }

    private void DoThat()
    {
        ISQLRepository repoB = repo.GetRepository();
        // now do something with repository B interface methods like this repoB.SomeMethod()
    }
}


I thought of an alternative way to do this but it has its own drawbacks as well.

-
Implement the interface methods in both IGenericeRepository and as well as the interface I inherited.

-
Implement the methods for inherited interface inside IGenericRepository and put it in each different region and end up with a massive page of code

Thoughts? Comments? Suggestion?
-- I personally think the first option is a better choice but wonder if there are better ideas or maybe there is something wrong with my implementation.

Alternative Code

```
public class GenericRepository : IGenericRepository, IFileRepository, ISQLRepository
{
public FileRepository fileRepo { get; set; }

Solution

-
Your current implementation is somewhat flawed as relying on ToString to get the type name is dangerous. ToString is meant largely to yield or short meaningful description of the object for debugging, logging etc.

-
typeof(T) already yields a Type - it is totally unnecessary to convert it to a string just to then turn it back into a type again.

-
If your repositories all have a parameterless default constructor you can avoid calling CreateInstance entirely.

Your current implementation can be shortened to:

public T GetRepository() 
{
    object repoInstance = Activator.CreateInstance(typeof(T));
    return (T)(object)repoInstance;
}


or possibly even to:

public T GetRepository() where T : new()
{
    return new T();
}


In addition to that your GenericRepository seems more like a repository factory than a repository in itself hence you should possibly rename it to RepositoryFactory and the method to CreateRepository.

Code Snippets

public T GetRepository<T>() 
{
    object repoInstance = Activator.CreateInstance(typeof(T));
    return (T)(object)repoInstance;
}
public T GetRepository<T>() where T : new()
{
    return new T();
}

Context

StackExchange Code Review Q#66782, answer score: 3

Revisions (0)

No revisions yet.