Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Today I had an argument with someone.

I was explaining the benefits of having a rich domain model as opposed to an anemic domain model. And I demoed my point with a simple class looking like that:

public class Employee
{
    public Employee(string firstName, string lastName)
    {
        FirstName = firstName;
        LastName = lastname;
    }

    public string FirstName { get private set; }
    public string LastName { get; private set;}
    public int CountPaidDaysOffGranted { get; private set;}

    public void AddPaidDaysOffGranted(int numberOfdays)
    {
        // Do stuff
    }
}

As he defended his anemic model approach, one of his arguments was: "I am a believer in SOLID. You are violating the single responsibility principle (SRP) as you are both representing data and performing logic in the same class."

I found this claim really surprising, as following this reasoning, any class having one property and one method violates the SRP, and therefore OOP in general is not SOLID, and functional programming is the only way to heaven.

I decided not to reply to his many arguments, but I am curious what the community thinks on this question.

If I had replied, I would have started by pointing to the paradox mentioned above, and then indicate that the SRP is highly dependent on the level of granularity you want to consider and that if you take it far enough, any class containing more than one property or one method violates it.

What would you have said?

Update: The example has been generously updated by guntbert to make the method more realistic and help us focus on the underlying discussion.

share|improve this question
17  
this class violates SRP not because it mixes data with logic, but because it has low cohesion - potential God object – gnat Jan 7 at 21:39
1  
What purpose is adding holidays to the employee? There should be perhaps a calendar class or something that has the holidays. I think your friend is right. – James Black Jan 8 at 1:48
7  
Never listen to somebody who says "I am a believer in X". – Stig Hemmer Jan 8 at 8:47
1  
About SRP, you might be interesting in this question I asked – Pierre Arlaud Jan 8 at 9:52
25  
It's not so much whether this is a violation of the SRP as whether it is good modeling at all. Suppose I am an employee. When I ask my manager if it's OK with him if I take a long weekend to go skiing, my manager does not add holidays to me. The code here does not match the reality it intends to model, and therefore is suspicious. – Eric Lippert Jan 8 at 15:31

11 Answers 11

up vote 63 down vote accepted

Single Responsibility should be understood as an abstraction of logical tasks in your system. A class should have the single responsibility to (do everything necessary in order to) perform one single, specific task. This can actually bring a lot into a well-designed class, depending on what the responsibility is. The class that runs your script engine, for example, can have a lot of methods and data involved in processing scripts.

Your coworker is focusing on the wrong thing. The question is not "what members does this class have?" but "what useful operation(s) does this class perform within the program?" Once that is understood, your domain model looks just fine.

share|improve this answer

Single Responsibility Principle is only concerned with whether or not a piece of code (in OOP, typically we're talking about classes) has responsibility over one piece of functionality. I think your friend saying that functions and data can't co-mingle didn't really understand that idea. If Employee were to also contain information about his workplace, how fast his car goes, and what type of food his dog eats then we'd have a problem.

Since this class deals only with an Employee, I think it's fair to say it doesn't violate SRP blatantly, but people are always going to have their own opinions.

One place where we might improve is to separate Employee information (like name, phone number, email) from his vacation. In my mind, this doesn't mean that methods and data cannot co-mingle, it just means that perhaps the vacation functionality could be in a separate place.

share|improve this answer

To my mind this class could potentially violate SRP if it continued to represent both an Employee and EmployeeHolidays.

As it is, and if it came to me for Peer Review, I'd probably let it through. If more Employee specific properties and methods were added, and more holiday specific properties were added I would probably advise a split, citing both SRP and ISP.

share|improve this answer
    
I agree. If the code is as simple as provided here, I would probably let it slide. But in my mind, I should not be the responsibility of the Employee to handle its own holiday. It might not seem like a big deal with where the responsibility is placed, but look at it this way: If you were new to the code base, and had to work on the holiday-specific functionality - where would you look first? For the holiday-logic, I personally would NOT look at the Employee entity to begin with. – Niklas H Jan 8 at 13:37
    
@NiklasH Agreed. Tho personally I wouldn't look randomly and try and guess the class I would search with studio for the word "Holiday" and see what classes it came up in. :) – NikolaiDante Jan 8 at 14:21
4  
True. But what if it is not called "Holiday" in this new system, but is "Vacation" or "Free Time". But I agree, with that you typically has the ability to just search for it, or can ask a co-worker. My comment were primarily to get the OP to mentally model the responsibility and where the most obvious place for the logic would be :-) – Niklas H Jan 8 at 14:30

There are already great answers pointing out that SRP is an abstract concept about logical functionality, but there are subtler points that I think are worth adding.

The first two letters in SOLID, SRP and OCP, are both about how your code changes in response to a change in requirements. My favorite definition of the SRP is: "a module/class/function should have only one reason to change." Arguing about likely reasons for your code to change is much more productive than arguing about whether your code is SOLID or not.

How many reasons does your Employee class have to change? I don't know, because I don't know the context in which you're using it, and I also can't see the future. What I can do is brainstorm possible changes based on what I've seen in the past, and you can subjectively rate how likely they are. If more than one scores between "reasonably likely" and "my code has already changed for that exact reason", then you are violating SRP against that kind of change. Here's one: someone with more than two names joins your company (or an architect reads this excellent W3C article). Here's another: your company changes how it allocates holiday days.

Notice that these reasons are equally valid even if you remove the AddHolidays method. Plenty of anemic domain models violate the SRP. Many of them are just database-tables-in-code, and it's very common for database tables to have 20+ reasons to change.

Here's something to chew on: would your Employee class change if your system needed to track employee salaries? Addresses? Emergency contact info? If you said "yes" (and "likely to happen") to two of those, then your class would be violating SRP even if had no code in it yet! SOLID is about processes and architecture as much as it's about code.

share|improve this answer

That the class represents data is not the responsibility of the class, it is a private implementation detail.

The class has one responsibility, to represent an employee. In this context, that means it presents some public API that provides you with functionality you need to deal with employees (whether AddHolidays is a good example is debatable).

The implementation is internal; it so happens that this needs some private variables and some logic. That doesn't mean that the class now has multiple responsibilities.

share|improve this answer
    
Interesting line of thinking, thanks a lot for sharing – reddy Jan 8 at 13:55
    
Nice - Good way to express the intended goals of OOP. – user949300 Jan 9 at 21:56

"I am a believer in SOLID. You are violating the single responsibility principle (SRP) as you are both representing data and performing logic in the same class."

Like the others, I disagree with this.

I would say that the SRP is violated if you are performing more than one piece of logic in the class. How much data need be stored within the class to achieve that single piece of logic is irrelevant.

share|improve this answer

Just to say it once: The idea that mixing logic and data in any way is always wrong, is so ridiculous it doesn't even merit discussion. Yet, there is a clear violation of single responsibility in the example, but it's not because there's a property DaysOfHolidays and a function AddHolidays(int).

It's because the employee's identity is intermingled with the holiday management, which is bad. The employee's identity is a complex thing that's required to track vacation, salary, overtime, to represent who's on which team, to link to performance reports, etc. An employee can also change first name, last name, or both, and stay the same employee. Employees may even have multiple spellings of their name such as an ASCII and a unicode spelling. People can have 0 to n first and/or last names. They may have different names in different jurisdictions. Tracking the identity of an employee is enough of a responsibility that holiday or vacation management cannot be added on top without calling it a second responsibility.

share|improve this answer
    
"Tracking the identity of an employee is enough of a responsibility that holiday or vacation management cannot be added on top without calling it a second responsibility." + Employee might have several name etc... The point of a model is to focus on the relevant facts of the real world for the problem at hand. There exists requirements for which this model is optimal. In these requirements, Employees are only interesting to the extent that we can modify their holidays and we're not interested too much in managing other aspects of their real life specifics. – reddy Jan 14 at 0:46
    
@reddy "Employees are only interesting to the extent that we can modify their holidays" - Which means you need to identify them correctly. As soon as you have an employee, they can change their last name at any time due to marriage or divorce. They can also change their name and gender. Will you fire employees if their lastname changes such that their name matches that of another employee? You will not add all of this functionality right now. Instead you'll add it when you need it, which is good. Irrespective of how much is implemented, the responsibility of identification stays the same. – Peter Jan 14 at 11:52

SRP is solely focused on interface design, of operations and not state. A microwave has a single logical responsibility of heating up food as a crude example. The internal state and mechanisms required to do that are irrelevant to SRP. It gets a little more complex when we boil down a single reason for something to change and the level of abstractions that might entail. Yet SRP in a nutshell is concerned with operations, and not the states required to perform them.

A counter-argument to your co-worker might be that functions can never be divorced from state, inside or outside of objects. A function which operates on a name field will need that field, whether or not such a field resides in an object.

One interesting phenomena that does arise with OOP if we compare it to functional programming is that object-oriented design revolves around encapsulation, which does potentially bundle up data in a way that might make a large amount of it irrelevant to a given member function. A class with 8 data members may have a method for which only one of those data members might be relevant to the context of the function.

In that sense, if we imagine such data floating around elsewhere with functions to operate on them, we can have such functions only receive the relevant data. For example, instead of this (which might be a useful way to look at OOP when separating state from operation):

void AddHolidays(Employee employee, int numberOfdays)

We might be able to do this:

void AddHolidays(List<int> holidays, int numberOfdays)

... and avoid passing in far more information than needed, which reduces coupling (at least from the standpoint of AddHolidays). Objects bundle state, and therefore have a tendency to expose more data than actually necessary to their methods.

From this facet, SRP also encourages the notion of reducing coupling even from this function/method-centric view, as an object that models a single responsibility will typically have less state and less methods accompanying it, and thus have "more local, more relevant" states accessible to each of its methods. A massive class with disparate responsibilities and a large number of private variables would have a rather "global" set of variables, as the visibility/scope of such private variables would get wider the more methods a class contains, and the more would share that level of visibility the more private variables exist.

A huge practical benefit of OOP is the ability for objects to maintain invariants, which cannot exist without information hiding, which cannot exist without encapsulation. The importance of maintaining invariants largely interacts with side effects. OOP often seeks to reduce the number of places in a system that can cause side effects which compromise invariant properties, by reducing the potential places that are capable of making changes to relevant states. Likewise OOP tends to reduce implicit coupling overall, since such data, should it simply float around outside the object, would have much wider visibility than usual, and all ability to maintain invariants would then be lost.

It is perhaps for this reason that pure functional languages lacking the concept of encapsulation tend to have a heavy focus on immutability. Invariants cannot be maintained without information hiding, but if we zoom out of invariants and start simply looking at the causes for human error, a lot revolve around side effects. Functional programming often seeks to mitigate side effects to minimum or zero by favoring immutability. OOP might be seen, from one small facet of side effects, as a way to mitigate the number of places allowed to cause side effects by preventing access to internal states to all but a handful of functions.

Anyway, this is getting into a whole side subject (though maybe interesting in considering exactly what happens when we lose encapsulation), but SRP doesn't focus on state. It's focused solely on functionality.

share|improve this answer

On the contrary, for me Anemic domain model breaks some OOP main concepts (which tie together attributes and behavior), but can be inevitable based on architectural choices. Anemic domains are easier to think, less organic and more sequential.

Many systems tend to do this when multiple layers must play with the same data (service layer, web layer, client layer, agents...).

It is easier to define the data structure in one place and the behavior in other service classes. If the same class was used on multiple layers, this one may grow big, and it asks the question of which layer is responsible to define the behavior it needs, and who is able to call the methods.

For example, it may not be a good idea than an Agent process which computes statistics on all your employees can call a compute for paid days. And the employee list GUI certainly not need at all the new aggregated id computation used in this statistic agent (and the technical data coming with it). When you segregate the methods this way, you generally end with a class with only data structures.

You can too easily serialize / deserialize the "object" data, or just some of them, or to another format (json)... without worrying about any object concept / responsibility. It is just data passing though. You can always do data mapping between two classes (Employee, EmployeeVO, EmployeeStatistic…) but what does Employee really mean here?

So yes it separates completely data in domain classes and data handling in service classes but it is here needed. Such system is at the same time a functional one to bring business value and a technical one too to propagate the data everywhere it is needed while keeping a proper responsibility scope (and distributed object does not solve this either).

If you don’t need to separate behavior scopes you are more free to put the methods in service classes or in domain classes, depending on how you see your object. I tend to see an object as the "real" concept, this naturally help to keep SRP. So in you example, it is more realist than the Employee's Boss add payday off granted to his PayDayAccount. An Employee is Hired by company, Works, can be Sick or be Asked for an advice, and he has a Payday account (the Boss may retrieve it directly from him or from a PayDayAccount registry...) But you can make an aggregated shortcut here for simplicity if you don't want too much complexity for a simple software.

share|improve this answer
    
Thanks for your input Vince. The point is, you don't need a service layer when you have a rich domain. There's only one class responsible for the behaviour, and it is your domain entity. The other layers (web layer, UI etc...) typically deal with DTOs, and ViewModels and this is fine. The domain model is to model the domain, not to do UI works, or send messages across the internet. Your message reflects this common misconception, which comes from the fact that people simply do not know how to fit OOP into their design. And I think this is very sad - for them. – reddy Jan 14 at 0:26
    
I don't think I have a misconception of rich domain OOP, I've designed a lot of sotwares that way, and it is true it is really good for maintenance / evolution. But I'm sorry to tell you this is not a silver bullet. – Vince Jan 14 at 10:07
    
I'm not saying it is. For writing a compiler it probably isn't, but for most of the line of business / SaaS applications, I think it is much less art/science than what you suggest. The need for domain model can be proven mathematically, your examples make me think of debatable designs rather than flaws of limitations in OOP – reddy Mar 24 at 12:01

You are violating the single responsibility principle (SRP) as you are both representing data and performing logic in the same class.

It sounds very reasonable for me. Model might not have public properties if exposes actions. It is basically a Command-Query Separation idea. Please note that Command will have private state for sure.

share|improve this answer

You cannot violate the Single Responsibility Principle because it is just an esthetic criterion, not a rule of Nature. Don't be misled by the scientifically sounding name and the uppercase letters.

share|improve this answer
1  
This is hardly a real answer, but it would have been great as a comment to the question. – Jay Elston Jan 26 at 2:08

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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