Lots of classes with only one single static method with same name as class - Code smell? [duplicate]

softwareengineering.stackexchange https://softwareengineering.stackexchange.com/questions/232145

Question

I'm trying to follow the single responsibility principle (SRP) in my applications. I have lots of CRUD classes I just name xxxxxManager.

Following the SRP, I made 4 classes for each one :

xxxxxCreator, xxxxxGetter, xxxxxDeleter, xxxxxUpdater

They end up having only one static method : xxxxxCreator::create(Model $model), xxxxUpdater::update(Model $model), etc.

This is a code smell for me. Am I pushing the SRP too far ?

Was it helpful?

Solution

The single responsibility is data access, not creating, reading, updating and deleting. Your granularity is too small at that level.

SRP doesn't mean that the class should only do one thing; it means that the class should have only one reason to change. The Wikipedia article describes it as

Every class should have a single responsibility, and that responsibility should be entirely encapsulated by the class. All its services should be narrowly aligned with that responsibility.

OTHER TIPS

What you're doing is writing functions, not classes. Not all languages allow stand-alone functions, but if the language allows you, it would make more sense to write them as functions.

This is not necessarily a bad thing. A lot of people like to dogmatically say "everything is an object", but they're wrong -- these things you're writing aren't objects, they're functions, even though they're wearing object clothing. There's nothing wrong with functions.

The SRP is OOP specific terminology, but the idea also works for procedural or functional programming; instead of an object or class having a single responsibility, a function does a single thing. These functions do a single thing.

However, if Model is an OO class or object (not just a data structure that happens to be an 'object' or 'class' in the type system of the language you're using), it might not mesh well with the style of the rest of the code.

If you want to do things in a more OO way, I would guess that you could put these in the Model class, since the two examples you give both take a Model as their only argument. Then, assuming that Model is not already huge, bloated, or unwieldy, it would likely be in line with the SRP.

I don't believe anywhere in SRP does it say that each class should have a single method... Also, you created 4 classes which have a lot of dependencies - changes in xxxxCreator will almost certainly come together with xxxxUpdater.

I would say that CRUD definitely falls within the realm of single dependency.

The single responsability principle main reasosn of existence are :

  • To prevent you to make classes who do everything in a total mess and
  • Force you to segregate your code in well separate concerns.

So, basically, if you make a class who handle the 4 CRUD actions on an entity, for me, it's fine. Moreover, I would add that your methods should respect the SRP principle too. For example, the update action should NOT return an entity, it's the role of the read action, not the update !

I'd say so. I think there was a little bit of talk back in the day about redesigning Java so that every class only had one method: run(). This is effectively what you're talking about. This didn't really get anywhere or pick up any traction anywhere else, because you couldn't really do much with it.

What you start running into at this point is a very hard time implmementing very basic control structures. Because if you think about what an if statement looks like:

if (condition)
{
    // do this
}
else
{
    // do that
}

is that just doing one thing? Is it just running()? No, it's doing two things, and it's not even the same one or two each time. Sometimes it doesThis(), and sometimes it doesThat(). So what you have there, with virtually any if statement with an else clause, is a choice:

  1. Use multiple functions within the class or module or whatever.

  2. Use only one function, but with distinctly different behavior in different spots that could each be easily converted into a separate function.

Is 1 cleaner than 2, or is 2 cleaner than 1? ...Yes. It really depends on the situation, with regards to which one you want to go with. What you do not want to do is rid yourself of if statements. And every non-infinite loop that's not automatically broken out of pretty much implies an if statement.

Why make a separate class for each updating, deleting, creating, and getting? Somewhere along the road, you'll still end up having to do this:

if (condition1)
{
    xxxxxCreator.Create();
}
else if (condition2)
{
    xxxxxGetter.Get();
}
else if (condition3)
{
    xxxxxUpdater.Update();
}
else
{
    xxxxxDeleter.Delete();
}

I count one, two, three, four different things that are being done all in the same class. Even if you break this up into smaller if statements, there will still be a conditional somewhere, even if it's in the machine code. ...And that violates such a strict interpretation of the necessity of a function/class needing to do one and only one thing.

Since you cannot forgo conditionals in programming, you should not try to adhere to such a strict interpretation. Go ahead and create an xxxxxManager class with methods Get(), Update(), Create(), and Delete().

Licensed under: CC-BY-SA with attribution
scroll top