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.

3
  • What's the gain of implementing the Service layer with static classes? Quite apart from this comment. Is It a own project? Are you working solo? If don't may be not everyone agree with your "conventions".
    – Laiv
    Apr 7, 2017 at 21:27
  • From what you've described, using static methods in classes that have zero state; and, not having unit tests for trivial functionality... are both good programming practices. Go with your instinct on this rather than following this generic rule.
    – Darius X.
    Apr 8, 2017 at 3:11

3 Answers 3

6

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.

8
  • 2
    Which is why it would be an ordinary class instead of a static one, because the ordinary class can hold state variables containing the service layer dependencies. Apr 8, 2017 at 2:53
  • @RobertHarvey Is composition the best/only way to go here, b/c I was considering just replacing all static calls like ElementService.Get(); with either (new ElementService()).Get() or by wrapping references in a "global" static class like GlobalService.ElementService.Get() (where ElementService would now be an instantiated object, not a static class). My goal is to save time, however if neither of these code factorizations support Unit Testing, then I'll try another approach. I'm trying to get to the crux of the matter without reading first, but I'm researching now).
    – samus
    Apr 21, 2017 at 13:15
  • 1
    @SamusArin: If all you're doing in these classes is CRUD (create, read, update, delete), it's not really a Service Layer anyway. Service Layers generally provide business functions, not database functions. Apr 21, 2017 at 13:48
  • @RobertHarvey It's a wrapper that handles details in reading/writing "Entities" (record objects) to their respective repositories. Sometimes its a simple call through (like in example), but often a few repositories are used to fulfill the requested operation. For example, saving a new record requires first generating a temporary record id (for local db table) before writing it, which involves the IdMap and LocalId repositories. Then an update request in written to a queue table which also involves a repository.
    – samus
    Apr 21, 2017 at 14:10
  • 1
    @SamusArin: It depends on the client application's requirements. Service layers provide whatever services the client application needs. CRUD is usually relegated to the Repository layer. For the client application, it's usually preferable to expose business actions in the Service Layer rather than CRUD (for a number of reasons), but that certainly doesn't preclude other services such as real-time communication endpoints. Apr 21, 2017 at 16:41
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.

3
  • Is it b/c statically scoped constructs are allocated outside of the stack and heap that make them non-OPP? Or is it simply b/c it's code that can be ran outside the context of instantiated class object (is it a compile-time or run-time violation?). From what I've read recently, arbitrary static methods calls made throughout a program create a web of interconnected dependencies somehow. I'm not sure why, need to give it some thought, but does make me wonder where objects instantiated within the body of a static method get allocated?
    – samus
    Apr 21, 2017 at 13:40
  • 1
    @SamusArin: Whether objects get allocated on the stack or the heap is an implementation detail in C# (i.e. you shouldn't be concerned about it). That said, reference types are generally created on the heap, not the stack. Apr 21, 2017 at 13:46
  • 1
    @samus no. try this exercise. make four projects, one with your service layer, one with your controllers, one with a hosting layer which references the controler and service layer projects and heres the kicker, one test project which references the controller project but NOT the service layer. the controllers should use a mock service layer. You will find you cant do it with a static service layer
    – Ewan
    Apr 21, 2017 at 14:09
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:

5
  • If the Service Layer is merely a set of self-contained methods, then you don't need OOP. See John Wu's answer for the real reason. Apr 8, 2017 at 2:51
  • Yes but you won't have SOLID code. I myself use static code from time to time. It is not bad as long as you understand the difference between OOP and not OOP Apr 8, 2017 at 2:55
  • SOLID is a set of OO principles. You can't be SOLID at all if you're programming in C. Apr 8, 2017 at 2:56
  • Exactly. But if you know that and still decide to use static code at a particular feature then probably it is not bad. However, see that c is not the same as static code. In c you still have abstractions by using pointer to functions. In static code you don't have any abstraction at all. Apr 8, 2017 at 2:59
  • I'm down with OOP.
    – samus
    Apr 10, 2017 at 14:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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