Item — HN Reader

Ok, so a ”Maybe” avoided, but instead we added a coupling, plus moved the decision logic to the inside of the Customer object?

I would consider this a very bad design. It’s one of those solutions that you think of and tell yourself ”look at how elegant it looks”. Then one month (or more) when you revisit the code to do some additions / code reading, you realize how bad an idea it was.

3 comments

I completely agree. This is a bad design and not something that I would allow on my team. That logic does not belong in the Customer object.

Where's the added coupling? Before you'd have two accessors returning their own type, one of them nullable. Now you have one method that doesn't return. That interface is also agnostic of how customers can be reached: If we add sendByPigeon() the Customer interface won't see change.

The customer object is in a good position to decide how the customer should be reached.

It's coupled the Customer to the service announcements without there being any need to. At least in the first refactoring.

The second refactoring is somewhat different. That would make more sense if the communication decision wasn't based solely on the existence of an email address. But all he's really done now is add a kind of bespoke Visitor pattern to work around a lack of double dispatch.

Personally, I'd be inclined to keep the naive version unless there were clear requirements for double dispatch (and probably more than one example). If there were multiple requirements, I'd build in an abstract Visitor framework so you're back to limited coupling at the expense of some complexity.

TBH I'd probably be using a language that had multiple dispatch but that's not always an option.

You're forgetting the calling code in your accounting: While the Customer objects learns about Announcements, the calling code (we don't see) forgets about EmailAddress and PostalAddress. That means less coupling overall.

I think this must be seen as a toy example. The Option type used this example, so it's natural that the "Tell, Don't Ask" would use the same, even though the option type is perhaps the better fit.