Mark Needham

Thoughts on Software Development

Coding: Coupling and Expressiveness

with 16 comments

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.

Be Sociable, Share!

Written by Mark Needham

August 25th, 2009 at 10:42 pm

Posted in Coding

Tagged with

  • CM

    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?

  • 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.

  • 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.

  • CM

    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 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.

  • CM

    best of luck and thanks for making me think! 🙂

  • 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.

  • 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.

  • 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?

  • You know, another way to cut down on people abusing this method, keeping down the coupling, and revealing the intent (maybe :P) 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.

  • Pingback: Twitter Trackbacks for Coding: Coupling and Expressiveness at Mark Needham [markhneedham.com] on Topsy.com()

  • Pingback: DotNetShoutout()

  • Pingback: Coding: Coupling and Expressiveness at Mark Needham | Coupling live today()

  • 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 🙂

  • Pingback: Coding: Reduce fields, delay calculations at Mark Needham()

  • Pingback: Coding: Connascence – Some examples at Mark Needham()