current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote -1 down vote favorite

My current implementation for service and business layer is straight forward as below.

public class MyEntity { }

// Business layer
public interface IBusiness { IList<MyEntity> GetEntities(); }
public class MyBusinessOne : IBusiness
{
    public IList<MyEntity> GetEntities()
    {
        return new List<MyEntity>();
    }
}

//factory
public static class Factory
{
    public static T Create<T>() where T :  class
    {
        return new MyBusinessOne() as T; // returns instance based on T
    }
}

//Service layer
public class MyService
{
    public IList<MyEntity> GetEntities()
    {
        return Factory.Create<IBusiness>().GetEntities();
    }
}

We needed some changes in current implementation. Reason being data grew over the time and service & client cannot handle the volume of data. we needed to implement pagination to the current service. We also expect some more features (like return fault when data is more that threshold, apply filters etc), so the design needs to be updated.

Following is my new proposal.

public interface IBusiness
{
    IList<MyEntity> GetEntities();
}

public interface IBehavior
{
    IEnumerable<T> Apply<T>(IEnumerable<T> data);
}

public abstract class MyBusiness
{
    protected List<IBehavior> Behaviors = new List<IBehavior>();
    public void AddBehavior(IBehavior behavior)
    {
        Behaviors.Add(behavior);
    }
}

public class PaginationBehavior : IBehavior
{
    public int PageSize = 10;
    public int PageNumber = 2;
    public IEnumerable<T> Apply<T>(IEnumerable<T> data)
    {
        //apply behavior here
        return data
            .Skip(PageNumber * PageSize)
            .Take(PageSize);
    }
}

public class MyEntity { }

public class MyBusinessOne : MyBusiness, IBusiness
{
    public IList<MyEntity> GetEntities()
    {
        IEnumerable<MyEntity> result = new List<MyEntity>();
        this.Behaviors.ForEach(rs =>
        {
            result = rs.Apply<MyEntity>(result);
        });
        return result.ToList();
    }
}

public static class Factory
{
    public static T Create<T>(List<IBehavior> behaviors) where T : class
    {
        // returns instance based on T
        var instance = new MyBusinessOne();
        behaviors.ForEach(rs => instance.AddBehavior(rs));
        return instance as T;
    }
}

public class MyService
{
    public IList<MyEntity> GetEntities(int currentPage)
    {
        List<IBehavior> behaviors = new List<IBehavior>() { 
            new PaginationBehavior() { PageNumber = currentPage, }
        };
        return Factory.Create<IBusiness>(behaviors).GetEntities();
    }
}

Experts please suggest me if my implementation is correct or I am over killing it. If it correct what design pattern it is - Decorator or Visitor.

Also my service returns JSON string. How can I use this behavior collections to serialize only selected properties rather than entire entity. List of properties comes from user as request. (Kind of column picker)

share|improve this question

closed as off-topic by Jamal♦ Jul 12 at 17:20

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Jamal
If this question can be reworded to fit the rules in the help center, please edit the question.
    
Is MyEntity really an empty class, or did you just remove the guts from it? –  Jeff Vanzella Feb 7 '13 at 18:37
    
This is just for brevity. –  hungryMind Feb 7 '13 at 18:41

1 Answer 1

up vote 1 down vote

Your original design is wrong. The whole idea of having a factory is to have an abstraction that is able to generate other entities, and in your case "factory" is static so your code still depends on actual instances created. It's the same as if your code manually creates those objects, just it is done in a separate class. Static factory doesn't allow you to unit test your business logic independently.

Why is your "Business" layer looks so similar to repository? And your factory looks more like a container than a factory... And why "service" just returns the entities?

Basically your original code should look like that:

public class MyEntity { }

// Repository - better use Entity Framework or NHibernate if connecting to DB
public interface IRepository<T>
{
    //TODO: Consider changing return type to IQueryable<T>
    IEnumerable<T> GetEntities();
}

public class MyRepository<T> : IRepository<T>
{
    public IEnumerable<T> GetEntities()
    {
        return new List<T>(); //TODO: populate instances here
    }
}

//Business layer
public class MyService
{
    private readonly IRepository<MyEntity> _repository;

    public MyService(IRepository<MyEntity> repository)
    {
        _repository = repository;
    }

    public IList<MyEntity> DoSomeBusinessLogic()
    {
        var myEntities = _repository.GetEntities().ToList();
        //do smth. with entities
        return myEntities;
    }
}

MyService should be wired up with proper instance of IRepository<T> by IoC framework (see Autofac, Unity or any other IoC framework).

Now to your improvements...

By applying "behavior" after you've downloaded all instances you just reduce the amount of data returned, but all entities are still loaded. Instead of doing so just add the ability to query paged data to repository. If GetEntities method returns IQueryable then you don't even need to change the repository, since repository clients would be able to filter that themselves. If you prefer to return IEnumerable<T> or IList<T> - then you have to introduce a new method to return paginated results:

// Repository - better use Entity Framework or NHibernate if connecting to DB
public interface IRepository<T>
{
    //TODO: Consider changing return type to IQueryable<T>
    IEnumerable<T> GetEntities();

    IEnumerable<T> GetPageEntities(int pageId, int pageSize);
}

public class MyRepository<T> : IRepository<T>
{
    public IEnumerable<T> GetEntities()
    {
        return new List<T>(); //TODO: populate instances here
    }

    public IEnumerable<T> GetPageEntities(int pageId, int pageSize)
    {
        return new List<T>(); //TODO: make sure not to load unnecessary entities
    }
}
share|improve this answer
    
I needed input on implementing behaviors on existing functioanlty. Please overlook namings, return types (I can get QueryAble or enumerable as data source is LINQ) and factory implementation. This is just dummy code and actual implemenation has proper factory and EF as data layer. –  hungryMind Feb 7 '13 at 18:43
    
I guess you managed to make your example so dummy that it's hard to see your actual design. If you return an IQueryable<T> from repository then just make an extension method static IQueryable<T> Paged(this IQueryable<T> query, int pageId, int pageSize) –  almaz Feb 7 '13 at 18:48
    
I personally would return IEnumerable and only IQueryable if there was a essential reason to do so. As hungryMind suggests you can always get IQueryable later if wished. –  dreza Feb 7 '13 at 18:50
    
@dreza can't say I agree with you... In your case you need to know that IEnumerable returned from repository is actually IQueryable in order not to loose the ability to translate queries to underlying LINQ storage. And that "hidden" knowledge is something I would not want to see in my code. (even though .AsQueryable will take care of that properly) –  almaz Feb 7 '13 at 19:01
    
I guess we are deviating from my understanding. My query was if this design is good to apply some behavior on result data (pagination was just an example, and there could be many more behaviors on different services) Definately my aim is to return LINQ expression to the service and EF will not execute untill serailizer at service layer begun serialization. –  hungryMind Feb 7 '13 at 19:16

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