3

I think it would be easiest to explain what I'm asking with an example.

function getLastNode() {

    let current = this.head;

    if (current == null) {
        // Here, we could either return current, or return null
    }

    while (current.getNext() != null) {
        current = current.getNext();
    }

    return current;
}

In the beginning of the function, in the if statement, we have a choice to either write the return statement as return current or return null. In situations like this you could choose either option since they are equivalent.

To avoid this question being considered primarily opinion-based, I want to know if there is any type of convention for this. Is there a standard, convention, or style guide that specifies a standard practice?

Thanks!

0
9

Two things to always consider:

  1. Consider which leaves your code most responsive to change

  2. Consider which is easier to read

1 might argue for current if the flag value might ever be something other than null, I don't see that happening myself.

2 argues for null because it says flat out: if you're here, you're returning null. Even makes life in the debugger easier.

On balance, I'll take return null here.

The views presented here should in no way be construed as an endorsement of null.

3
  • 1
    I'd also side with (2) .. to me, the test is to detect a "(somewhat) abnormal state" and the result in such cases should be null. The fact that current happens to be null in the current test is (almost) accidental, and so should not be relied on. (A future "abnormal state" test might be if( current == null || current.isInvalid() ) return null) – TripeHound Oct 12 '16 at 8:26
  • @TripeHound a very good point. That's actually a point in favor of being responsive to change. – candied_orange Oct 12 '16 at 8:30
  • @CandiedOrange Yes, I'm coming from a "responsive to change" P.O.V.; I probably should have said "I also side with return null" -- the (2) in my original comment was referring to the "_2 argues for null..." part, rather than the "2...easier to read" part. – TripeHound Oct 12 '16 at 10:56
3

Instead of having a separate return for the empty list, I'd just put the while loop into an if(current != null) { block and then return current unconditionally.

It's not always necessary to avoid multiple returns in a method, but if it's possible and even makes the code more succinct, you should do it.

2
  • 2
    Avoid multiple returns when your resources don't throw exceptions (say in c). Otherwise beware of heading down the path to the arrow anti pattern. – candied_orange Oct 12 '16 at 8:07
  • 1
    If you "just put the while loop into an if(current != null) { block", you now have an extra level of nesting. Not saying you are wrong. but I'd argue that returning early is better practice than creating extra layers of nesting. – David Arno Oct 12 '16 at 10:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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