Coding: Coupling and Expressiveness
We came across an interesting situation in our code base recently whereby two coding approaches which I consider important for writing maintainable code seemed to come into conflict with each other.
The code we were working on needed to retrieve some customer details from a backend system by making use of the current user's 'customerId' which we can retrieve from the 'LoggedInUser'.
My initial thought was that since we only needed one property of the 'LoggedInUser' we could just pass in the 'customerId' instead of the 'LoggedInUser':
public class Repository { public Customer RetrieveCustomer(string customerId) { var backEndSystemCustomer = backEndSystem.RetrieveCustomer(customerId); return MapCustomer(backEndSystemCustomer); } }
Which we would use like this:
public class SomeController { ... public ActionResult DoSomething() { repository.RetrieveCustomer(loggedInUser.CustomerId); // and so on } }
I recently came across quite a nice post which explains different types of cohesion and coupling that we might find in our code and from my understanding the above code has data coupling which is the loosest type of coupling that we can have apart from message coupling:
Data coupling is 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).
It seemed to me that it was better to couple the repository to the data it required rather than to the 'LoggedInUser' which would be stamp coupling:
Stamp coupling (Data-structured coupling) is 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). This may lead to changing the way a module reads a record because a field, which the module doesn't need, has been modified.
In this case I was thinking about coupling of classes instead of coupling of modules (perhaps wrongly?)
Discussing this with Dave, he pointed out that the method wasn't really very expressive and that it is actually possible to pass in any string we want – even one that might not even be a customerId. The chance of making a mistake when using this API is quite high.
We therefore changed the method signature so that it takes in a 'LoggedInUser' instead and then just takes the 'customerId' from that object.
We only build a 'LoggedInUser' from one place in our application and everyone on the team knows that so it's much less likely that someone would make a mistake and pass in the wrong instance.
public class Repository { public Customer RetrieveCustomer(LoggedInUser user) { var backEndSystemCustomer = backEndSystem.RetrieveCustomer(user.CustomerId); return MapCustomer(backEndSystemCustomer); } }
I think the code is definitely nicer like this although LoggedInUser and Repository are now coupled even though the repository only cares about one property of LoggedInUser.
It seems to me though that the main reason we care about coupling is that loosely coupling our code makes it easier to change but on the other hand making our code more expressive makes it easier to read which is also important for making it easy to change.
Maybe it's not such a big deal anyway – I just found it interesting that I thought I'd done the right thing and it turned out that a way I had previously rejected turned out to be more appropriate.
Could you just use a value object which wraps the string and gives it a context eg SystemUserName or something better named with a constructor that takes the customer id string and has an overloaded toString() method to show the value?
CM
25 Aug 09 at 11:39 pm
Yeh I guess that would be another way to do it although I'm never sure what the correct way to make use of 'toString' is – is it ok to use it to return a value?
We do want to ensure that you can only make that repository call if you are a LoggedInUser to prevent calls being made to get the wrong customer's details for example which I think means we would still want to have the LoggedInUser as the type in the method signature.
Mark Needham
25 Aug 09 at 11:48 pm
This is a great observation. The degree to which our components are coupled is expressive of their relationships. If we always strive for the loosest coupling, the intent of the code can be lost.
kofno
25 Aug 09 at 11:48 pm
I think the toString part is a red herring. Forget it and lookup however value objects are done in DDD. I think the value object is the way to go though. Using the value object still keeps some of the intent. How are you going to prevent someone creating a LoggedInUser object?
CM
26 Aug 09 at 12:28 am
@CM yeh I suppose that's true – when we were discussing this one suggestion was that you could setup NDepend or something to ensure that a LoggedInUser was only created from certain places in the code. We haven't looked much into that though.
Will have a look at the value object idea though, thanks.
Mark Needham
26 Aug 09 at 12:32 am
best of luck and thanks for making me think!
CM
26 Aug 09 at 12:41 am
Cross-layer coupling is bad, but a certain amount of coupling among classes and interfaces *within* the domain layer is expected. And since the interface of a repository is generally considered to be part of the domain, I tend to think that coupling the repository interface to a domain object such as a LoggedInUser is perfectly natural.
Chris Ammerman
26 Aug 09 at 1:28 am
What jumped out at me when I first looked at the code was that I've gotten yelled at plenty for passing around primitives. So I can't help but ask, why not pass a CustomerId object instead of a String? The CustomerId could simply hold the string, but it would be clearer from the API perspective what the method wants, and you'd cut down on the coupling.
Zach Shaw
26 Aug 09 at 4:37 am
I was thinking "TinyType!" while reading this. I think CM and Zach have already suggested essentially a TinyType. Or is TinyType significantly different from a class with one field?
Dave Cameron
26 Aug 09 at 7:22 am
You know, another way to cut down on people abusing this method, keeping down the coupling, and revealing the intent (maybe
) could also be to invert the control of the Retrieve method, it might be a little ugly, but you could do something like:
void Retrieve(CustomerReader customerReader) {
id = customerReader.getCustomerId();
//look stuff up
customerReader.setCustomer(theCustomerWeFound);
}
Where LoggedInUser implemented CustomerReader.
If I was presented with this method I would look for types that implemented CustomerReader to see who I should be passing into the method, and probably not implementing my own type to pass in.
Zach Shaw
26 Aug 09 at 8:24 am
[...] Coding: Coupling and Expressiveness at Mark Needham http://www.markhneedham.com/blog/2009/08/25/coding-coupling-and-expressiveness – view page – cached Thoughts on Software Development — From the page [...]
Twitter Trackbacks for Coding: Coupling and Expressiveness at Mark Needham [markhneedham.com] on Topsy.com
26 Aug 09 at 10:39 am
Coding: Coupling and Expressiveness – Mark Needham…
Thank you for submitting this cool story – Trackback from DotNetShoutout…
DotNetShoutout
27 Aug 09 at 11:49 pm
[...] connector which is the loosest identify of connector that we crapper … Excerpt from: Coding: Coupling and Expressiveness at Mark Needham Posted in Uncategorized | Tags: a-nice-post, and-from, cohesion-and, different-types, [...]
Coding: Coupling and Expressiveness at Mark Needham | Coupling live today
28 Aug 09 at 3:54 pm
I was definitely thinking TinyType too.
I wouldn't worry about LoggedInUser being created elsewhere. The best kind of system is the one that is simple enough to be used in previously unthought ways
Andy Palmer
1 Sep 09 at 8:09 pm
[...] 'SomeObject' and 'Dependency1′ and 'Dependency2′ by storing those in fields and I was pondering the wisdom of doing this sort of thing in a previous post but I think I prefer to take this coupling over the alternative [...]
Coding: Reduce fields, delay calculations at Mark Needham
2 Sep 09 at 11:54 pm
[...] think this principal is quite similar to the idea of coupling which we seem to use more frequently these days but I found it quite compelling that as I was [...]
Coding: Connascence – Some examples at Mark Needham
28 Oct 09 at 10:44 pm