views:

412

answers:

10

I'm just reading Code Complete by Steve McConell and I'm thinking of an Example he gives in a section about loose coupling. It's about the interface of a method that calculates the number of holidays for an employee, which is calculated from the entry date of the employee and her sales. The author suggests a to have entry date and sales as the parameters of the method instead of an instance of the employee:

int holidays(Date entryDate, Number sales)

instead of

int holidays(Employee emp)

The argument is that this decouples the client of the method because it does not need to know anything about the Employee class.

Two things came to my mind:

  1. Providing all the parameters that are needed for the calculation breaks encapsulation. It shows the internals of the method on how it computes the result.

  2. It's harder to change, e.g. when someone decides that also the age of the employee should be included in the calculation. One would have to change the signature.

What's your opinion?

+1  A: 

1) Providing parameters doesn't break encapsulation. It just shows that these parameters are used to calculate holidays. The "HOW" is still hidden inside the method.

2) Holiday method should not be a part of Employee class.

shahkalpesh
For #2, I don't think he is suggesting that the holidays method is a part of the Employee class.
BobbyShaftoe
Sorry about assuming that. I have yet to read a good book like "code complete"
shahkalpesh
It's been a long time since I've read, I think I may pick it up again.
BobbyShaftoe
+1  A: 
  1. This is not breaking encapsulation. Breaking encapsulation would be revealing the internal methods it uses to calculate the holidays. Providing the starting parameters is what defines the API.

  2. The API could be improved to allow such changes - however, most approaches suggest that you should design to meet the requirements have currently and not to over design for unforeseen changes (design for change, but do not try to predict the change). It's better to implement what you need now and refactor later if necessary.

In my opinion it's best to plan for change by decoupling and encapsulating as much as possible so that refactoring could be done as easily as possible. Attempting to predict ahead of time every possible scenario ends up with a bloated over-designed system.

Eran Galperin
#2 is what I hear from many agile propagandists and I worry that that is just too short-sighted.
BobbyShaftoe
It's not short-sighted, it's just pragmatic. After you learn first hand what you need by coding it, you can start improving the design if necessary.
Eran Galperin
But does it still seem pragmatic when you are in a situation where you need to modify the API? I mean, a lot of the arguments in the agile world claim pragmatism but I wonder if it really is.
BobbyShaftoe
consider the case that the API never changed. You either put the effort upfront, or divide incrementally over time - saving some where no changes were needed
Eran Galperin
Also, how can you know ahead of time to what will the API evolve to? as others have mentioned, it's not necessarily the Employee object that will be required.
Eran Galperin
#2 is not a valid argument for an Agile "propagandist". Agile only works because it always keeps the design well decoupled and encapsulated, and thereby keeps the cost of change low. So, worrying about coupling and encapsulation is *not* overdesign, but *necessary* to be Agile.
Ilja Preuß
I highly recommend the book "Agile Software Development - Principles, Patterns and Practices" by Robert C. Martin on the topic. (There also is a variant of the book available that uses C# instead of Java as the language of choice.)
Ilja Preuß
You completely misunderstood. I said the current API might be improved but it's not a given, and it's designed well enough for change. Read my last paragraph and see that I said exactly what you iterated here.
Eran Galperin
A: 

I would say that one of the major benefits of loose coupling is the ease of change. Loosely coupled types can change independently of each other so I don't understand your "vs" in the question.

Additionally, I would not say that you break encapsulation by providing parameters for the method. You can implement a Sum(int a, int b) anyway you want - but you have to tell me (as the user) that you expect two numbers.

Brian Rasmussen
+1  A: 
int holidays(Employee emp)

In this case only an employee can use the function in question...

Chuck Conway
Well, but his point is that if you need to change what values are necessary to calculate holidays, then the signature would need to be changed. However, if another sort of object could have "holidays" calculated then you could add an overloaded method, perhaps. I think it's an interesting question.
BobbyShaftoe
+5  A: 

The problems I see with your argument number 2 are

  1. you are assuming every needed value comes from an Employee instance. This is by no means always true. For example, say you have to consider the financial state of the company to calculate how much 'bonus holiday' give to any employee. Would you add financial state information to the employee class to avoid changing the signature?

  2. changing a signature is not necessarily "harder", especially so in these days of tools that will highlight every calling place at the click of a button.

And the main problem with your argument number 1 is that it just doesn't break encapsulation as everyone else has said. You are showing the what, not the how, which is what encapsulation is about.

Vinko Vrsalovic
A: 

Unlike some other posts i agree with you that this breaks encapsulation. Not the encapsulation of a class, but of the concept of calculating holidays. If you want to change how this calculation works in the future, and still avoid tight coupling, i would suggest making Employee an interface. If this is still too tightly coupled to Employee because you think holidays might need to be calculated for other things, then you could have a GetsHolidays interface from which Employee inherits.

Of course, how involved the solution you choose is should depend on how serious you consider the coupling and encapsulation problems to be. I would consider both the original solutions to be acceptable in many situations, keep it simple if possible.

+3  A: 

Ultimately loose coupling wins. From high coupling to low, there are various classes of coupling:

  1. Content coupling (high): One module modifies or relies on the internal workings of another module
  2. Common coupling: two modules share the same global data (e.g. a global variable). Changing the shared resource implies changing all the modules using it.
  3. External coupling: two modules share an externally imposed data format, communication protocol, or device interface.
  4. Control coupling: one module controlling the logic of another, by passing it information on what to do (e.g. passing a what-to-do flag).
  5. Stamp coupling (Data-structured coupling): when modules share a composite data structure and use only a part of it, possibly a different part (e.g. passing a whole record to a function which only needs one field of it).
  6. Data coupling: when modules share data through, for example, parameters. Each datum is an elementary piece, and these are the only data which are shared (e.g. passing an integer to a function which computes a square root).
  7. Message coupling (low): Modules are not dependent on each other, instead they use a public interface to exchange parameter-less messages (or events, see Message passing).
  8. No coupling: Modules do not communicate at all with one another.

Passing in Employee is Stamp coupling, which is more coupled than Data coupling. If you really think about the ease of modification, low coupling is better because you have less to worry about the unwanted side effects. Suppose you want to change the structure of the Employee class. You now have to check the actual implementation of the holidays function to make sure that it doesn't break the math.

The best most decoupled way is to define an interface IHolidayable, but that's an overkill for this situation.

eed3si9n
A: 

The best most decoupled way is to define an interface IHolidayable

This sort of blanket statement really annoys me. Just because you use an interface does NOT mean you automatically decouple anything. If your employee class implements an interface to calculate holidays, any code that calls the method, still calls the same method. At worst, the calling code will do that by direct access to an employee object, not an IHolidayable interface reference, in which case you have only made the situation worse (because there is now also a more subtle coupling between the interface and any classes which implement it).

Interfaces can indeed help with decoupling, but they do not automatically do so, and even when they do, they are not necessarily a better solution than an abstract (or some other ancestor) class.

Jim Cooper
I hope you don't think I meant that the interface is a silver bullet for every situation. Also you have omitted my second half of the sentence "but that's an overkill for this situation." Passing an minimally defined interface instead of the native object is a form of Message coupling.
eed3si9n
+1  A: 

To me the "right" answer boils down to whether you're defining a high-level or low-level API when creating this function. A low-level API puts flexibility above convenience for any particular case and argues for int holidays(Date entryDate, Number sales) . A high-level API is designed to do one thing well with as much convenience as possible for the client. This argues for int holidays(Employee emp) because it requires less boilerplate on the caller's part (extracting the date and sales from the Employee class) and is less verbose.

dsimcha
A: 
int holidays (IHolidayInfo obj)

Is might be a way. In this case, any object that implements IHolidayInfo would able to use "holidays" function. Just an alternative.

Veritte