Re: Episode 19 Advanced TDD mistakes

[Uncle Bob]

Point 5 is interesting.  It implies that we should throw some kind of exception instead of returning a blank.  We certainly don't want to pass null around since that will cause NPEs at some unrelated point.  So perhaps we should create NameInverter.InvalidNameException.

[Nicole Bekkers]

I agree on not passing null around, but passing an empty string instead is interesting.

In this case I can imagine that the NameInverter class is part of something bigger, where the name comes from user input. I would test the validation of my input for null, empty string and even the required format before trying to invert the name. In that case I don't need to throw an exception or return an empty string in the NameInverter class, but also making sure no invalid data gets passed around.

However when the NameInverter is going to be a public library on the internet for everyone to use (as a helper method) I would take a more defensive approach and throw a custom Exception that explains the problem. InvalidNameException would be very nice! :)

For an external system I don't know the details of I would use the same approach as user input and check whatever comes from there first.

Replying to this was a good exercise for myself. But I would love to hear how others think about this.

[Marcel Gabriel]

I agree with your point 5. What I also don't like:

1. usage of default method accessors (I'm no java dev but doesn't that make all methods of NameInverter public (at least within the package))? If this has been done for testability of all methods of the NameInverterClass then I'd argue that most of those methods are implementation details that should be hidden inside the NameInverter class using the private accessor and that tests should be run against the public interface of this class (i.e. "invertName")

2. method names: If the class name is called NameInverter, then I'd rather call the method "invertName" something like "invoke" or "invert" (DRY principle - a name inverter inverts names so why repeat that in a method name);