Representative Line: All the Small Things

Kerry (previously) has a long held belief: people that can't get the little things right have no hope of getting the big things right, and not just when it comes to software.

Personally, I don't think that's truly universal, but it's certainly a good guideline. If nothing else, seeing certain simple mistakes gives you a hint of worse things to come. Like this interface definition:

long? saveEntry(CreateEntryViewModel dto, out long? IncidentNumber, out int incidentCount, string IncidentOrigination, bool ForceMilesAtPurchase);

Speaking of small stuff", one of the smallest things we might expect from a method signature is consistent capitalization. Out of the gate, we fail on that- most of the parameters are PascalCase, but one camelCase entry sneaks in. The method returns a value, but it also has two out parameters- output parameters for when you have a method which doesn't return a value. The return type and one of those outputs is nullable, why? Well, there's no documentation, so who knows? We also sandwich our out params between more traditional input params, which as a general rule, we don't want to mix up our outputs and our inputs. It's unhealthy.

Now, there is no documentation, as I mentioned. But that doesn't mean that Kerry hasn't been able to figure out what some of this method does. First, while it's called saveEntry, it only creates them. There's a separate updateEntry, which is only mildly confusing. There's no indication anywhere of how the output parameters are actually meant to be consumed, and from the handful of places this is called, it looks like they simply aren't.

But the return value is used, and it is important. This method returns the entryId of the newly created entry- the unique ID of this entry. Well, maybe it does, because the return type is nullable- long?. There's just one problem with that: the entryId should never be null. A null is an exception- you failed to save the entry. This method should never return null, it should just throw an exception.

For one line of code, that's a lot of bad choices. Each one of them is just a small thing, but this interface's implementation needs to do big, complicated things on the back end. As Kerry adds:

Needless to say, I'm apprehensive as to what I will find upon opening the service's implementation.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!