Question

In my team, we work closely with a few software architects. They approve all design decisions of our projects, do some code reviews etc.

Our projects consist mainly of backend functionality implemented in PHP using the Symfony 2 framework. So syntactically, the code, naming conventions and project structure look almost identical to what Java would look like (Symfony 2 encourages such structure). I'm mentioning this because Java-specific conventions also apply in our case (if possible).

Recently, they suggested something that I find very strange: all methods should have conjunctions in their name e.g. getEntityOrNull, setValueOrException etc.

Such a naming convention feels very wrong to me, but I can't come up with any concrete arguments or online articles/pages that specifically challenge this.

The only things I came up with is:

  • such information should be present in the method's annotations, like @return or @throws
  • the use of conjunctions ("and", "or" etc.) in method names usually suggest that the Single Responsibility Principle is not properly respected

What are some other concrete arguments against this naming convention?

Was it helpful?

Solution

They are probably doing so because of naming conflicts. I assume that you cannot have two methods named getEntity, one which possibly throws an exception and one which returns null. Therefore, you have to name them accordingly.

I, for one, dislike the practice of having many different ways of calling the same method unless it is performing some alteration that only that particular class can perform.

In other words, if getValueOrNull is simply calling getValueOrException, capturing the exception, and in that case returning null, this can be performed by the caller. It clutters the class with methods that don't really contribute anything useful. Rather I would prefer the getValue, and know that it throws the exception. Better still, I would prefer to know that all get methods potentially throw exceptions, and none of them return null instead so that behavior is uniform throughout my project, or inversely, all potentially return null, and know that the caller would have to throw an exception if that were desired.

However, it is also true that you're not in charge of that. My advice is to bring it up, but don't sweat the petty things. Ultimately, the responsibility of such naming conventions falls on their shoulders, regardless of whether or not they take your advice, and because it is their asses on the line, I believe it is their prerogative to say no, in my humble opinion.

OTHER TIPS

Although the use of conjunctions often indicates a violation of the SRP, in this case it is just indicating the return value of a poor man's algebraic data type that can be one of two values, either null or a "success" value.

They are using a kind of Hungarian Notation to compensate for weaknesses in the type system, namely a lack of non-nullable types. In other words, there is no way to specify in your @return annotation that the function will never return null, and conversely, there's no way to specify in your @return annotation that the function may possibly return null.

Also, I believe that @throws annotations are not mandatory in php, so the absence of the annotation doesn't indicate the absence of an exception, although that's better solved by making the annotation mandatory in your style guide.

Given those limitations of the language you're using, it's not an entirely unreasonable style.

In my code I sometime create method pairs with names like getEntity() and getEntityOrNull(). The name makes the expected behavior clear. If getEntity() finds no entity then an exception is thrown. getEntityOrNull() will return a null.

By doing this, the calling code becomes a bit clearer. If the calling code has to have an entity then getEntity will do the trick. If entity is some sort of optional thingee then getEntityOrNull is the method of choice.

The same thing could be accomplished with a single method but that then shifts some of the burden to the calling program. You always need to test for a null or you need a try/catch block. Either way it's extra code that needs to be duplicated each time the method is called.

If your architects are not using these sorts of method pairs then yes, I would question the need for the suffix.

Never really saw a setValueOrException method. Can't think of a good common use case for that.

You might consider asking the architects why. The ones I know are always glad to explain their decisions. Often in great (and sometimes excruciating) detail.

Your two arguments are sound. But if they're the ones in charge of deciding the naming convention, try not to feel too bad about it being something you don't agree with.

While you're at it, you should convince them not to use the "get" and "set" portions unless those methods actually do directly set or get a member variable.

Licensed under: CC-BY-SA with attribution
scroll top