10

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:

  1. 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.

samus
  • 475

3 Answers3

7

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).

Sure, but that's not the issue. The issue is whether you are able to isolate away the dependency on the service layer when unit testing the layer immediately above, e.g. your ElementController. If you can't, then all of your unit tests will require a database in order to be run, and you will be unable to tell if any failed tests are due to a flaw in the controller or a flaw in some layer lower down in the service stack, including incorrect data in the database itself.

John Wu
  • 26,955
4

The reason we have these 'Golden Rules' and principals like YAGNI and SOLID is because they sum up things where argument for/against is complicated but the general principal will keep the inexperienced programmer out of trouble.

In the case of 'Dont use static' obviously its not always wrong to use it or maybe you can ignore the downsides for expedience. But if you dont understand WHY the rule is there then the best advice is to follow it.

In my view the predominant reason for this rule is due to OOP. devs would start to write OOP code, find that they had no reference to an instance of an object with a general method which they wanted to call, and solve the problem by making the method static.

Although this solves the immediate issue, its not OOP. Students would ask 'but why isnt it OOP?' and the simple answer, 'because you have statics' is given. Hence the rule.

In your particular case though, the practical downside (amongst others) of making your service layer static is that you cant mock it and unit test your controllers (hosting layer i would call it)

For example, personally, I would normally create a Mock service layer which returned results from static files, inject this into my web api/controllers and use the resulting site in the integration tests of components which consumed the api.

This is not possible with a static service layer as you can't compile the controllers without the static, real implementation, of the service layer.

Ewan
  • 83,178
1

The piece of code that contains or uses static classes/methods is not OOP. You won't benefit from any of the OOP advantages. To use OOP or not is solely your decision but before tacking that decision you should understand what you are loosing by understanding well what OOP means.

If you already know then make up your mind by puting in balance what you loose and what you gain (simpler code, time, etc).

If you don't know then read more here: