views:

234

answers:

4

In my ASP.NET MVC application, I have a project that contains all the business logic/service layer. This project interacts with my Database (Entity framework) which is located in a separate project.

I wanted easy access to the service layer, so I created static classes in it so they can be referenced easily. For example, if I'm in my controller and I need to create a new account:

 ServiceLayer.Accounts.CreateAccount(userName, passWord) //etc..

The service layer then does all the required logic, then creates the user via the repository in the DatabaseLayer.

    private static AllRepos _Repos;
    private static AllRepos Repos { 
       get 
        { 
           if(_Repos == null)
              _Repos = new AllRepos();

           return _Repos
        }
    }

    public static void CreateAccount(string username, password)
    {
        string salt = GenerateSalt();
        Account newAccount = DatabaseLayer.Models.Account
              { 
              Name = username,
              Password = HashPassword(password, salt),
              Salt = salt
              };
        Repos.AddAccount(newAccount);      
    }

Because I didn't want to do the following everywhere in my service layer:

 AccountRepository Accounts = new DatabaseLayer.AccountRepository();

I instead created a wrapper class for my repositories so that I only have to instantiate it once to use all the other repositories.

 public class AllRepos
 {

    private AccountRepository _Accounts;

    public AccountRepository Accounts
    {
        get
        {
            if (_Accounts== null)
                _Accounts= new AccountRepository();

            return _Accounts;
        }
    }

    // the same is done for every other repository (currently have about 10+)
  }

Which was used in the static classes of the service layer.

Because all my service layer classes are static and the Repos field is also static, the obvious issue I keep encountering is where the same object is retrieved from multiple datacontexts causing weird behaviors for updates/deletions.

I understand that this is to be expected if I use static members/classes as I did since they last the lifecycle of the application, but is there a way to be able to use the service layer as ServiceLayer.Accounts.Method() without having to create a non-static class which needs to be instantiated everywhere it is used and not encounter the CRUD issues due to multiple datacontext instances?

+3  A: 

I'm not sure why you're so dead-set against using instances. Aside from the fact that the code you have now is unnecessarily complicated, using static types also makes it harder to unit test. The static type is like a singleton which you can't mock/replace. It seems to me that your real question might be, "How do I lifetime-manage instances of my service layer?" Typically, you do this by having one instance per Web request. In an ASP.NET MVC application, you can new up a DI container via a ControllerFactory, and handle all of this. [PDF]

Craig Stuntz
+3  A: 

Your approach to this is really not a recommended one. Personally I would never allow this type of approach on my team. The drawbacks:

  1. Major threading issues, as you are experiencing, which are not simple to resolve
  2. You cannot test this very easily at all.
  3. Unrealistic abstraction of your data access

The biggest reason for creating instances of repositories is so that you can inject the dependencies if you need to. The most well-known argument for this is for unit testing so you can mock the dependencies, but I've built a number of repositories that have interface dependencies that change in production code.

You should simply be making your repository instantiation as part of your base service classes anyway. There's no reason that it has to be "static OR instance calls everywhere". There should be limited instance instantiation code inside base classes.

womp
+1  A: 

You need to handle the scope of your objectcontext in a deliberate way, like doing a Unit Of Work pattern

Apart from that, I do think you should reconsider doing everything static, as womp says you get very high coupling, and makes it very difficult to test, and you can get a lot of help with managing the dependency graph by using a IOC container.

I can say this, having burnt myself doing that kind of thing previously :)

Luhmann
A: 

Singletons (and static classes as described) are evil and should be avoided at all costs especially in Web Apps.

Ahmad
i don't agree with that, Helper Methods are ideal when static.infact some of the best helper methods are static for web use. Request, Server.MapPath, File/Directory etc...
Mike