Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single exit point #5

Open
jbgi opened this issue Feb 11, 2016 · 6 comments
Open

Single exit point #5

jbgi opened this issue Feb 11, 2016 · 6 comments

Comments

@jbgi
Copy link
Contributor

jbgi commented Feb 11, 2016

(this is duplicated from gitbook discussion, because not sure where most people watch the project)

I like most of the guidelines of the book, however I find the judgment on "Single exit point" style overly harsh.

Single exit point is effectively mandated in most (all?) functional languages. Are you sure that Haskell "works at a lower level than Java"?

The "Single exit point" example is said "Overly complex", I don't really agree, but anyway the example is not fair. A good single return point style would be one of the method of gist.github.com/jbgi/c60acbfd7870237d0de6

The advantages of single return point associated with a final result variable (or no result variable) are two fold:

  1. you ensure proper case analysis. (you can easily forget one "return" in the case of multiple return style and then it silently slip through, like when you forget a break in a switch)
  2. you can easily extract part of the method into another: you cannot do that if the part of the method you want to extract contains a return: a single return point method can easily scale when more cases are added, a multiple return method cannot.

Can the overly harsh judgment be corrected ? or the section be removed all together ?

@hcoles
Copy link
Contributor

hcoles commented Feb 11, 2016

Thanks for the feedback - yes this is the preferred place to comment. Unfortunately it doesn't seem to be possible to disable discussion over at gitbook.

The section around applying to "lower level languages" is badly thought out and will be updated.

I would still argue that trying to apply a single exit point rule in Java is harmful.

It makes sense for something like Haskell where everything is an expression. In Java most things are statements.

In your gist you've avoid the bloat with the ? operator i.e by using expressions.

If Java didn't have the ? operator we could end the discussion here . . . but of course it does.

If you tell a Java programmer to avoid multiple exits my experience is that the majority will write the bloat version.

You could tell them to avoid multiple exits but always do it using the ? operator. You will likely have to keep repeating the 2nd part of that advice, but even then the ? version has it's own issue.

It's overly terse.

  public int lookMaNoLocalVariable(int x) {
    return x <= 0 ? 0
         : x == 10 ? -value
         : value + 10;
  }

This is subjective, but I'd argue that the multi return version is easier to comprehend.

  public int multiExit(int x) {
    if (x <= 0) {
      return 0;
    }

    if (x == 10) {
      return -value;
    }

    return value + 10;  
  }

You might disagree over which one you find preferable, but I don't think you can reasonably argue that the multi return version is less easy to comprehend. We shouldn't be recomending against it on those grounds.

Regarding your point on refactoring.

In our contrived example refactoring is equally easy in both implementations (all you can really extract are the boolean expressions).

You might find examples in large complex methods where refactoring the multi exit version is harder, but the problem there is the large methods rather than multi exit.

As for accidently missing returns. Yes this could happen, but :-

  1. It is unlikely when methods are kept short
  2. A unit test should fail
  3. Static analysis will catch empty blocks after a conditional

An equivalent criticism of the ? based version would be that it is possible to get the operands the wrong way round. This is no more a reason to avoid ? than missing returns is to avoid multi exit.

There is one other drawback to the ? approach - it forces premature method extraction.

If you need a guard statement in a method you can start with a multi exit version that has both the guard and the guarded logic. If the guarded logic is small it may be most readable if it stays that way. If it grows it can be refactored out to another method.

You can't do this with ? - you have to seperate the two immediately.

None of this to say that a single exit functions using ? are not sometimes be the better implementation.

The point is that this is not universally true.

So . . .

Having said all that I think your criticism that it is overly harsh is still still correct and the section would benefit from some discussion of the ? operator.

@jbgi
Copy link
Contributor Author

jbgi commented Feb 11, 2016

I would argue for expressions, even in java.
The ternary operator is not the only way, you can also use Optional/Either/... monads.

as for

  1. It is unlikely when methods are kept short

If the method are kept short then surely they cannot be a big difference between

  public int totallyComplexForSure(int x) {
    final int result;
    if (x <= 0) {
      result = 0;
    } else if (x == 10) {
      result = -value;
    } else {
      result = value + 10;
    }
    return result;
  }

and

  public int simpler(int x) {
    if (x <= 0) {
      return 0;
    }
    if (x == 10) {
      return -value;
    }
    return value + x;
  }

So then the two style should not really matter. For a bit more complex method then, yes, I hope that

A unit test should fail

if you screw up. But as the guide correctly state, tighter feedback (by the compiler) is even better. And we all known that sometimes unit test are lacking whereas well-typed expression are proved by the compiler.

Static analysis will catch empty blocks after a conditional

Not if the block is not empty (does some side effects), and is just missing the return.

So I would suggest to least add something like "Multiple-return, however may not be preferred if the team has chosen a functional programming approach: then, single end point methods would be most common and natural"

hcoles pushed a commit that referenced this issue Feb 12, 2016
@hcoles
Copy link
Contributor

hcoles commented Feb 12, 2016

The point of this is not whether single points are good/bad/generally preferable, but whether or not trying to mandate them in all circumstances is a good idea.

I've made some updates to the section in this commit based on this discussion.

@jbgi
Copy link
Contributor Author

jbgi commented Feb 12, 2016

Thanks, the updated version is better, but I would still like it better with a note that the single exit principle only make sense (bring additional guarantees in term of proper case analysis) if the result variable is final. and that the first example is actually a pointless/partial application of the rule. and that a correct application of the rule would be:

  public int correctSingleExit(int x) {
    final int retVal;
    if (x <= 0) {
      retVal = 0;
    } else if (x == 10) {
      retVal = -value;
    } else {
      retVal = value + 10;
    }
    return retVal;
  }

@jbgi
Copy link
Contributor Author

jbgi commented May 20, 2016

Much better now. I still do not agree with the "Not really." though: the multi-exit point is inferior in a way: you can forgot to do a return, while in the correct single exit you cannot forgot to affect the variable (otherwise it does not compile), I think it should be mentioned to be fair.

@jbgi
Copy link
Contributor Author

jbgi commented May 20, 2016

To be complete I would add the following rule of thumb: "If you cannot make the return variable final then a single-exit implementation has no particular interest, do not bother going for it"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants