tags:

views:

205

answers:

3

If I break my Objects down to 'Single Responsibilities', is there a fundamental thought whether like objects should live together or separately, for example if I have

class Employee_DataProvider() : IEmployee_DataProvider { ... };
class Employee_Details() : IEmployee_Details { ... };
class Employee_Payroll() : IPayroll() { ... };
class Employee_LeaveProcessing() : ILeaveProcessing_Client { ... };
...

Is it bad smell to have all these living inside, yet loosely coupled to through interfaces, an owning Employee class:

class Employee
{
    IEmployee_DataProvider _dataProvider;
    IEmployee_Details _details;
    IPayroll _payroll;
    ILeaveProcessing_Client _leaveProcessing;

    //My functions call the interfaces above

}

or is the thinking more around keeping these classes completely separate (or as least as separate as is posible) in the code? Or are both these methods a valid usage of SRP?

EDIT: I do not want critique on the feasibility of the object given in the example, I just made it up to illustrate the question. I agree data, leave and payroll processing are not the domain of the employee class.

It does appear though that SRP is asking me to move away from the object as real world representation to object as a properties and methods around a single functional concept

A: 

In your example, the question you have to ask yourself is should an Employee object know about its underlying dataProvider? I don't think it should.

An Employee should not have knowledge of leave processing; that should be the resposibility of the LeaveProcessing object, which might take an Employee as a parameter. Ditto, for Payroll.

Mitch Wheat
Are you critiquing my example, or answering the question that, no, I shouldn't be wrapping my like classes together?
johnc
I was answering your question. Not sure I could make it clearer? which bit was unclear?
Mitch Wheat
It wasnt clear whether you were saying that the example was a crap object disregarding SRP (on reflection I'd agree), or that in general, when responsibilities are separated, I should be able to treat each object as stand alone. It's not about the employee class above, rather the principle
johnc
+4  A: 

Go back to OOP basics: the Employee object should have methods that reflect what it does, not what is done to it.

Mark Brittingham
+1: All that extra stuff isn't part of Employee. It's part of some application that touches Employee.
S.Lott
Does ' do not want critique on the feasibility of the object given in the example' mean anything?
johnc
+1  A: 

Although no one has yet addressed my actual question regarding the principle itself, rather than the somewhat poor example I gave, of an Employee object knowing too much about the processes that affect it, for those who are obviously interested in the question (there are 2 'favourites' stars) I've done a bit more further reading, although I was hoping for more of a discussion.

I think what the two current answers are trying to say is that responsibilities separated should be able to be stand alone, and that my example was a perfect example of what not to do. I am more than happy to accept that.

There is a paragraph from ObjectMentor (Uncle Bob's site) that has an example that combines Connection and DataReader objects (two objects previously living in a modem class then separated out) into a ModemImplementation, but states

However, notice that I have recoupled the two responsibilities into a single ModemImplementation class. This is not desirable, but it may be necessary. There are often reasons, having to do with the details of the hardware or OS, that force us to couple things that we’d rather not couple. However, by separating their interfaces we have decoupled the concepts as far as the rest of the application is concerned.

We may view the ModemImplementation class is a kludge, or a wart; however, notice that all dependencies flow away from it. Nobody need depend upon this class. Nobody except main needs to know that it exists. Thus, we’ve put the ugly bit behind a fence. It’s ugliness need not leak out and pollute the rest of the application.

I think the line 'notice that all dependencies flow away from it' is an important one here

johnc