Storing state in service layer

I have a base service class, and I have a 开发者_StackOverflow中文版bunch of services, is it good/bad practice to store data on a service class?

For example:

public interface IFunkyService
{
  void AddComment(int quoteid, string comment);
  void SetProirirty(int quoteid, Importance proirity);
}

public class FunkyService : CustomServiceBase, IFunkyService
{

 private readonly IRepo _repo;
 private readonly IUserSession _user;
 public FunkyService(IRepo repo, IUserSession user)
 {
  _repo = repo;
  _user = user;
 }


     public void SetProirirty(int quoteid, Important priority)
     {

      //get quote object then persists

     } 

    public void AddComment(int quoteid, string comment)
     {

      //get quote object then persists

     }

}

Can I simply just have a private method that stores the quote object on the class? e.g.

private Quote _quote {get;set;} //store on the class???

private void GetQuote(int quoteid)
{
 _quote = _repo.single(quoteid); //or whatever
}

---

Note that the value in the class is only going to live as long as the service object itself. A service object is created and destroyed with every request to the service, so Quote will only live during the length of a single request. The only purpose I can see for something like that is a per-request cache (i.e., during a single request you reference the Quote object five times. You should only need to actually look it up from the backing store once).

1. Client makes request to server
2. Server instantiates FunkyService class
3. Client calls GetQuote
4. Server populates Quote in the class.
5. Client completes call.
6. Server finalizes request, disposes FunkyService object from (2).
7. The value of Quote is no longer stored in the class because the object is gone.

Edit: It appears the reason you want to do this is so that retrieving a quote object is done in one place, but it doesn't get called over and over again (i.e., making multiple requests to the database when only one is needed). You can implement the cacheable property design pattern to have a per-request cache without using a class variable. It would look something like this:

private Dictionary<int, Quote> _quoteCache = 
    new Dictionary<int, Quote>(); // cache storage - NEVER ACCESS DIRECTLY

public Quote GetQuote(int quoteid)
{
    // cache is invalid, so populate
    if (!_quoteCache.ContainsKey(quoteid))
    {
        _quoteCache.Add(quoteid, _repo.single(quoteid));
    }

    // return back to caller
    return _quoteCache[quoteid];
}

In the example above, the cache stores each unique quoteid retrieved from the database. So if you call GetQuote(5) five times in a row, it is only retrieved from the database via _repo a single time. However if you call GetQuote(6) then it goes to the database again because that quote is not available from the cache. After that, both 5 and 6 are still stored in the cache until the service request is finished and disposed.

Edit2: In the example you provided:

var t = GetTags(_quote);
// then do something with t, I may pass to another method:
if(IsClosed(_quote)){} 

Instead of referencing a class variable, have your repository return a Quote object and pass that reference along, like this:

private Quote GetQuote(int quoteid)
{
    return _repo.single(quoteid); //or whatever
}

// other code
var q = GetQuote(quoteid); // get the quote once
var t = GetTags(q);  // q doesn't have to retrieve from repo again
if (IsClosed(q)) {}
// etc.

---

Not sure if this applies in your case but when switching a stateless service (or generally any component) to being statefull you need to consider a few issues:

1. Thread Safety - your must make the state immutable of ensure thread safe access to that state. Also keep in mind that most tracking ( state tracking ) ORMs don't do so well when same instances are accessed from multiple threads.
2. Consider the possibility that the service is deployed in multiple instances (say for load balancing) in the future. If the state is not immutable you can end up with out of sync instances for the same entity.
3. If the reason for doing this is Caching and performance improvements you can design an orthogonal caching strategy that can avoid the above problems - ex using an NHibernate cache provider or caching data in the application layer.

As a rule of thumb try to keep as much as possible of your code stateless. It's safer to work with, allows you to improve performance by doing async operations and I also think it's easier for others to understand it. When you need to keep state, especially shared state make sure all access to it is thread safe and that is very well documented.

---

The problem with being stateful in your services is that the lifespan of the Service class instance needs to be considered.

For example, if your service is exposed via WCF, then the default instancing mode of 'per call' will negate any state between method calls, as each call will mean that a new service instance is created for each call from the client. However, you could use InstanceContextMode.PerSession to 'keep' the connection between client and service up between calls, which would then give you some benefit of the state, although it also has the downside that scalability can be limited as your client ultimately now has control over server resources used in your state.

@mellamokb's point is great, in that one of the common uses of state in SOA is caching.