11

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?

|improve this question
  • generally, you can't – gnat Sep 8 '14 at 8:07
  • 5
    the use of conjunctions ("and", "or" etc.) in method names usually suggest that the Single Responsibility Principle is not properly respected This is not the case for the examples you listed, where the conjunction is used to clarify the mechanism used to handle failures, not to indicate it may do one thing or another. Even the most narrowly-defined function may have legitimate failure conditions, e.g. popping an empty stack. – Doval Sep 8 '14 at 15:04
  • 4
    Consider: (1) using "optional" rather than "or null". "GetOptionalEntity" sounds better to my ear than "GetEntityOrNull". (2) the .NET base class library uses the convention that if you have two methods that differ only in what they throw, name the one that cannot throw "TryBlah". For example, Int32.TryParse and Int32.Parse -- both parse a string into an integer, but the former returns a Boolean indicating success and the latter throws on failure. – Eric Lippert Sep 8 '14 at 17:04
  • I wouldn't use a suffix for the throwing variant, but I would use one for the null returning variant. Could be Try..., ...OrNull, ...OrDefault. @EricLippert That's not the only convention in .net. Consider Single vs. SingleOrDefault, which is very close to the OrNull the OP suggested. – CodesInChaos Sep 9 '14 at 14:26
11

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.

|improve this answer
  • If you had to choose only one, it's better to return null (or better yet, an Optional/Maybe type) since throwing is relatively expensive. Writing a helper that checks if a value is invalid and throws doesn't add much overhead. However when you want the no-throw version, swallowing the exception won't give you back the performance hit you took throwing it. – Doval Sep 8 '14 at 18:05
  • 1
    @Doval Good point, and perhaps more importantly, exceptions should be exceptional. If you're often throwing exceptions, you're not using them right. Only problem with returning null is that null may actually be a valid return in some instances and so you would have to deviate from the norm and throw an exception in such instances. – Neil Sep 8 '14 at 20:11
3

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.

|improve this answer
2

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.

|improve this answer
  • That getEntity will throw an exception on failure is not at all clear to me, I would expect a simple NULL. – Elise van Looij Dec 15 '16 at 11:10
1

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.

|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.