I structured my "service" layers (or, what I thought to be service-like functionality) as static classes where each class is a grouping of complementary functions that together provide a cohesive set of operations for supporting the consuming layers' needs.
public static class ElementService
{
public static Element GetElementByAtomicNumber(int atomic_number)
{
ElementRepositorySQL elementRepositorySql = new ElementRepositorySQL();
return elementRepositorySql.Read(atomic_number);
}
}
For example, controllers use the service layer to read/write to a repository instead of dealing with it directly.
public class ElementController
{
public ElementModel Model {get; set;}
public void LoadModel(int atomic_number)
{
Model = new ElementModel();
Element e = ElementService.GetElementByAtomicNumber(atomic_number);
Model.Name = e.Name.ToUpper();
Model.Weight = Math.Round(e.AtomicWeight,4).ToString();
}
}
10 Golden Rules Of Good OOP, rule #5 states:
- Avoid Static Classes for Helpers, Utilities & C
Unfortunately, static classes act frequently as a global state and create the non-determinism that should be avoided. But it gets worse. Since static classes can be used everywhere in the code without being passed explicitly as parameters, they create secret dependencies that are not revealed by the API documentation.
I don't see how this would be much of a problem if the calls to the service are strictly done within the layer it was designed for? I have a convention where only controllers call the service layer; that particular set of services support controllers.
Last, but not least, code using static classes is not testable in isolation, making unit testing a nightmare.
Unless strictly needed for performance reasons, the use of static classes should be avoided. Static variables are still OK for constant objects (although a static property without setter would be better) or to hold private references to objects inside factory classes.
The majority of the functionality these services provide is so straight forward I think unit testing would just be overkill (read/write record by id).
Considering these circumstances, would it still be worth refactoring the static class implementation? It's a decent chunk of code, so I'd need a comparable RoI.