20

I've got a problem which can be simplified to this:

parameters: a, b

if (a > 5)
{
    Print("Very well, a > 5");

    if (b > 7)
        Print("Even better, b > 7");
    else
    {
        Print("I don't like your variables");
    }
}
else
{
    Print("I don't like your variables");
}

I would like to use only one else instead of two since they are the same code. What I thought of was creating an additional method, which will return combined true`false`, but this is a serious overkill.

Another option would be a goto, but this would make code less readable and unsafe.

What is the way to do it, avoiding checking the same condition many times and making it as readable as possible?

Michał
  • 2,202
  • 2
  • 17
  • 33
  • 3
    There's no "magic" way of doing this. You're going to have to choose one of those "overkill" options: create a method, declare a flag, encapsulate it in a lambda/anonymouse method, etc. – Kirk Woll Mar 24 '14 at 23:02
  • I've _long_ wanted a language feature (in some language) that would help with this, even before Java was invented. So far nobody has ever accomplished this, though. I usually use a `boolean`. – ajb Mar 24 '14 at 23:04
  • 1
    @ajb: How would you differentiate that feature from actually just wanting to do one `else` at the outer loop? Which `if` inside the code which `else` part you want to be unified? What if I use `switch` statement inside? Will it work if the inner `if`s are refactored to another function? What happen if an exception is raised? If you need to mark each `if` with special marker to indicate that you want the `else` part of this `if` to be taken care outside, then that would be the same as setting a flag. – justhalf Mar 25 '14 at 04:35
  • 6
    You can either obfuscate your code and jump through hoops to avoid goto because some college professor told you once that it was bad, or you can express what you intend using the well defined features of the language. One way is substantially more harmful to readability than the other. Dogma has little place in pragmatic programming. – James Greenhalgh Mar 25 '14 at 08:20
  • Have a look at use of [Ternary operator](http://stackoverflow.com/a/22628588/1686291) – Not a bug Mar 25 '14 at 08:57
  • `goto did_like;` is actually the most readable option here. – Radiodef Mar 25 '14 at 10:05
  • 1
    Why was my question put on hold? It asks for a solution for a **concrete problem**. Code may seem general, but it **is meant to simplify my question**; putting all of my application logic here is inappropriate. And also - the fact that there are multiple correct answers to a question doesn't make it opinion based. I provide ways to solve the problem, but I say why I find them bad. That's why I ask for solution and get different correct answers, is it that bad? **Please vote for reopen or state reasons for closing**. – Michał Mar 25 '14 at 14:40
  • @JamesGreenhalgh The OP included Java as one of the tags, and Java does not support `goto`. So at the very least, some solution will be needed for Java users reading this question and desiring solutions. – ajb Mar 25 '14 at 14:54
  • @ajb check out ruby's throw/catch mechanism – Niklas B. Mar 25 '14 at 14:57
  • @justhalf If you're asking about the first part of my comment--yeah, yeah, yeah, I know there are problems, it was just sort of wishful thinking. Still searching for the Holy Grail of a programming language where we can express our thinking without all this extra rigmarole :) :) :) Which I'm sure will happen sometime after we figure out how to write a proof-of-termination algorithm. – ajb Mar 25 '14 at 14:59
  • @ajb Or Haskell's `EitherT`.. I think you just haven't looked hard enough – Niklas B. Mar 25 '14 at 15:00
  • @Michael your comment "and we all know goto is bad" put the nail on this one I think. You know a solution but ask for alternative, more elegant solutions. How can that *not* be opinion-based? – Niklas B. Mar 25 '14 at 15:04
  • I don't understand why this question was put on hold either. He has a particular goal, to rewrite this code without using `goto` while eliminating duplicate code and extra checks. True there may be multiple solutions as is often the case but they are still correct based on facts. – Chris Drew Mar 25 '14 at 15:23
  • @NiklasB. Corrected this to show what I meant more precisely. It is *not* opinion based because I ask for *elegant* solution since one that is *not elegant* is unacceptable for me. – Michał Mar 25 '14 at 15:23
  • Asking for elegant solutions is generally off-topic I would argue because everyone has strong and diverging opinions about elegance – Niklas B. Mar 25 '14 at 15:25
  • @ChrisDrew According to your argumentation, it also fits the "This question has too many possible answers" part of "Too broad". – Niklas B. Mar 25 '14 at 15:30
  • @Kirk Woll Or he could use goto, which actually doesn't make the code unreadable or unsafe in this case. – flarn2006 Nov 21 '19 at 00:03

16 Answers16

22
void doILikeYourVariables(int a, int b) {
  if (a > 5) {
    Print("Very well, a > 5");
    if (b > 7) {
      Print("Even better, b > 7");
      return;
    }
  }
  Print("I don't like your variables");
}
Chris Drew
  • 14,926
  • 3
  • 34
  • 54
  • 1
    This is an option, but still `return` somewhere inside is pretty much equivalent to `goto`. It just translates it into `return` language – Michał Mar 24 '14 at 23:13
  • 14
    @Michał mutliple `return` statements is much easier to read than `goto` And it is pretty safe if you are in the habit of using RAII. – Chris Drew Mar 24 '14 at 23:23
  • 1
    You are right, I haven't considered `goto` safety. And readability - I don't mind a `goto` sometimes, but still it is considered a bad habit. Thanks :) – Michał Mar 24 '14 at 23:25
  • 1
    In your code if a > 5 but b <= 7 it will dislike my variables, which is not what the author of the question wanted in his example – NothingsImpossible Mar 25 '14 at 03:28
  • 4
    @NothingsImpossible: As you post your misunderstanding in multiple answer, I'll explain in each of your comment. In OP's code, having `a>5 and b<=7` will result in "I don't like your variables" after "Very well, a > 5" – justhalf Mar 25 '14 at 04:31
  • Thanks, the only answer who takes the straightforward route instead of introducing awkward logic just to avoid the jump for whatever reason – Niklas B. Mar 25 '14 at 05:50
  • @justhalf Wait a sec while I bury my head in the ground, brb – NothingsImpossible Mar 25 '14 at 10:17
  • 1
    @Michał you do realise that at the end of your `if (a > 5)` and before your corresponding `else` there is a hidden `goto` that says "goto after the end of the else"? – Seph Mar 25 '14 at 10:52
15

Boolean logic 101:

public void test(int a, int b, int c) {
    boolean good = true;
    if (good = good && a > 5) {
        System.out.println("Very well, a > 5");
    }
    if (good = good && b > 7) {
        System.out.println("Even better, b > 7");
    }
    if (good = good && c > 13) {
        System.out.println("Even better, c > 13");
    }
    // Have as many conditions as you need, and then
    if (!good) {
        System.out.println("I don't like your variables");
    }
}

Alternatively - if you want loads of checks -

enum Tests {
    A_gt_5 {
        @Override
        boolean test(int a, int b, int c) {
            return a > 5;
        }
    },
    B_gt_7 {
        @Override
        boolean test(int a, int b, int c) {
            return b > 7;
        }
    },
    C_gt_13 {
        @Override
        boolean test(int a, int b, int c) {
            return c > 13;
        }
    };

    abstract boolean test (int a, int b, int c);
}

public void test(int a, int b, int c) {
    boolean good = true;
    for ( Tests t : Tests.values() ) {
        good = good && t.test(a, b, c);
        if (!good) {
            break;
        }
    }
    if (!good) {
        System.out.println("I don't like your variables");
    }
}
cjhines
  • 1,148
  • 3
  • 16
  • 32
OldCurmudgeon
  • 64,482
  • 16
  • 119
  • 213
  • `// Duplicare possint in infinitum` you mean *and so on*? – Michał Mar 25 '14 at 00:02
  • @Michał - *Latin* -> *English* : *Duplicare possint in infinitum* -> *Duplicate indefinitely* – OldCurmudgeon Mar 25 '14 at 00:07
  • 2
    Isn't writing `// Duplicate indefinitely` more clear? :) – Michał Mar 25 '14 at 00:09
  • 1
    @Michał - yes - but not so much fun. I considered *add hock* (which is a joke referring to *ad hoc*) and *etc.* and *ad infinitum* but eventually decided to make you think. :) – OldCurmudgeon Mar 25 '14 at 00:11
  • 1
    The best answer of all. The most readable hands down, and raises no performance questions. +1. Talking about the first block of code though (the small and readable one). The second is the Java Enterprise version, and I think it is not that relevant, because IMO the author is asking in a more conceptual way, the question is even tagged with multiple languages. – NothingsImpossible Mar 25 '14 at 03:34
  • 2
    This is incorrect as it does not mirror the functionality of the original example. In the case where `a>5` and `b<=7`, the behavior of original is to print `"Very well, a > 5"`, then print `"I don't like your variables"`. – KevinZ Apr 21 '14 at 05:03
3
if (a > 5)
{
    Print("Very well, a > 5");
}
if(a > 5 && b >7)
{
    Print("Even better, b > 7");
}
else
{
    Print("I don't like your variables");
}

or

bool isEvenBetter = false;
if (a > 5)
{
    Print("Very well, a > 5");
    isEvenBetter = b > 7;
}
if(isEvenBetter)
{
    Print("Even better, b > 7");
}
else
{
    Print("I don't like your variables");
}
Mert Akcakaya
  • 3,109
  • 2
  • 31
  • 42
3

Actually for your case, there is only one instance where you "like" the variables, which is when a>5 and b>7. In that case, you just need to set a flag in the innermost if. Like this:

parameters: a, b

boolean good = false;
if (a > 5){
    Print("Very well, a > 5");
    if (b > 7){
        Print("Even better, b > 7");
        good = true;
    }
}
if(!good){
    Print("I don't like your variables");
}

Which is only additional one line (and one variable) while removing the multiple else (actually there is even no else!)


Side note

I would say that using goto (or anything that looks like it that is supported) in this case is acceptable.

I agree that "unconstrained use of goto" is bad, since it confuses the program flow, but on some situations, it's best to use goto, like the case you describe.

See this question: GOTO still considered harmful?

Actually, if you think about it, raising a (probably custom) exception is the same as goto, because it would make the program flow jump to a certain point (which is the catch or except).

What is the best reason not to use goto? It is because there might be multiple ways to enter a line of code. That's the same reason why multiple returns is not recommended by some people. But in your case, we indeed want to exit at multiple points, and hence that feature is required.

A constrained goto (such as exception handling in Java, which can only do "goto" to a "catch" line) is good.

I'm not saying that you should use goto, but I'm addressing the point where you say "goto is bad", and at the same time contributing to the pool of answers.

Community
  • 1
  • 1
justhalf
  • 8,960
  • 3
  • 47
  • 74
2

You might alter a state:

bool good = a > 5;
if(good)
{
    Print("Very well, a > 5");
    good = b > 7;
    if(good) {
        Print("Even better, b > 7");
    }
}
if( ! good) {
    Print("I don't like your variables");
}
2

How about using a do with breaks. This is actually a sneaky way of doing a goto though you can think of it as a filter composed of several if statements in which the default is the last bit if none of the if statements are hit.

parameters: a, b

do {
  if (a > 5)
  {
    Print("Very well, a > 5");

    if (b > 7)
    {
        Print("Even better, b > 7");
        break;
    }
  }

  Print("I don't like your variables");
} while (false);

EDIT - On repurposing language

A number of people have objected to this specialized use of a do while to solve a particular problem. The main objection seems to be that it appears to be a loop but isn't really a loop so this construct falls into a kind of uncanny valley of loop use. In other words, "It just ain't natural."

And I agree that it is an uncommon usage and there really should be comments to identify that this is a filter using a do while to create a block allowing the use of break statements whenever a decision tree branch end point is reached. That is really what this is, a hard coded forward traversal decision tree without backtracking made up of a series of decisions. At any point in the decision process we can break out with a decision or fall through with a default decision including indicating no decision at all.

One could say that any source that requires comments to be understandable is not good code. On the other hand, the reason almost all programming languages have some way of inserting comments is because annotating source code is extremely helpful when you come back six months later to make a change.

The nice thing about this approach is that it creates a local scope so that variables required during the decision process can be constructed and destructed properly.

In some ways it is somewhat like a lambda, to which I doubt anyone would object and it can be used in languages that do not support a lambda. In another way it is some what similar to a try catch.

Perusing the web one can find quite a few articles in which someone uses a programming language in a way different from it's original design intent such as this article on using C++ in a functional programming style or this online book on using object oriented practices with the C programming language.

All programmers have certain styles or language use habits. One good thing that can come from source code reviews and reading the source code of others is learning about a different way of using a programming language.

This is not tricky code like one would find as an entry to the Obfuscated C Programming contest. It is quite straightforward.

Edit: Better than a goto?

One question about this unusual use of a do while is, "Why not just use a goto?" Reading Dijkstra's essay of Go To Statement Considered Harmful, as well as this blog discussion on the essay and the goto statement, we can see there are several nice characteristics about using a loop with break statements which are not characteristics of a goto and its associated label.

The main characteristic, especially with this example is the one way flow in which there is a definite beginning and a definite end. There is no danger of inadvertently changing the program flow by moving the goto label. There is no danger of somewhere else in the function using the goto label as an opportune place to jump creating a dependency that was not originally intended. Reading the code, every programmer knows that where there is a break, you are leaving the loop and that on leaving the loop, you go to the source line after the loop close. The result is that you have a nice clean knowledge chunk, something that can be labeled as "figure out the text to print"

Richard Chambers
  • 16,643
  • 4
  • 81
  • 106
  • This is pretty sneaky - I don't need to make any additional method or lambda or whatever to achieve my goal. +1 – Michał Mar 25 '14 at 00:10
  • in your code , if a > 5 but b <= 7, it will dislike my variables, which the example code in the question doesn't do - so this is not quite right yet – NothingsImpossible Mar 25 '14 at 03:32
  • 1
    @NothingsImpossible: As you post your misunderstanding in multiple answer, I'll explain in each of your comment. In OP's code, having `a>5 and b<=7` will result in "I don't like your variables" after "Very well, a > 5" – justhalf Mar 25 '14 at 04:32
  • @justhalf Wait a sec while I bury my head in the ground, brb – NothingsImpossible Mar 25 '14 at 10:16
  • 1
    Don't do confusing constructs like loops not actually being loops. Just use `goto` instead. –  Mar 25 '14 at 10:38
2

Looking at all the answers, I would write it this way, and keep both elses. It makes no sense to complicate it.

parameters: a, b

if (a > 5)
{
    Print("Very well, a > 5");

    if (b > 7)
        Print("Even better, b > 7");
    else
        DontLikeIt();
}
else
{
    DontLikeIt();
}

And have a method, DontLikeIt(), that prints the response you want.

cederlof
  • 7,206
  • 4
  • 45
  • 62
  • Are you kidding !!!! Two else are not removed and another method also added !!! [Read the question again dude.](http://stackoverflow.com/q/22622332/1686291) :p :p and FYI, I am not the downvoter. – Not a bug Mar 25 '14 at 14:01
  • 2
    No, I'm not kidding. I argue for keeping both elses due to readability, and introduce an extra method to minimize code duplication. My conclusion is that the OP has a larger code-base than this small sample. – cederlof Mar 27 '14 at 09:04
1

Just for fun!

class VariableLiker {
  private:
    bool isGood_;
  public:
    VariableLiker() : isGood_(false) {}
    void checkA(int a) {
      if (a > 5) {
        Print("Very well, a > 5");
        isGood_ = true;
      }
    }
    void checkB(int b){
      if (isGood_ && b > 7)
        Print("Even better, b > 7");
      else
        Print("I don't like your variables");
    }
};

//...

VariableLiker variableLiker;
variableLiker.checkA(a);
variableLiker.checkB(b);
Chris Drew
  • 14,926
  • 3
  • 34
  • 54
  • 1
    Bad API. Your API suggests that you can also reverse the order of calling the checks, but in reality you can't. – André Mar 25 '14 at 08:53
  • @André, Agreed, clearly a class is a bit overkill for this and it needs expanding to be more robust. I was just pointing out that if you have to maintain state, it might make sense to encapsulate the state in a class and have methods on the class that use that state. Probably not in this case but I thought it was worth noting. – Chris Drew Mar 25 '14 at 09:01
1

Another way without break or goto is:

int c = (new[] { a > 5 ,a > 5 && b > 7 }).Count(x=>x);
if (c > 0)
{
    Print("Very well, a > 5 ");
}
if (c > 1)
{
    Print("Even better, b > 7");
}
else
{
    Print("I don't like your variables");
}
Zaheer Ahmed
  • 28,160
  • 11
  • 74
  • 110
1

The interesting aspect of your code is that even if it likes a, it "doesn't like your variables" if b is then not good enough.

Obviously you're not actually concerned about having two else statements in the code; it's the duplication of the "don't like" code that you seek to avoid. The following will do the trick and is useful if you don't mind throwing away the value of b.

if (a > 5)
    Print("Very well, a > 5");
else
    b = 0;

if (b > 7)
    Print("Even better, b > 7");
else
    Print("I don't like your variables");

If you need to retain the value of b then then you can use an additional variable.

var evenBetter = (b > 7);
if (a > 5)
    Print("Very well, a > 5");
else
    evenBetter = false;

if (evenBetter)
    Print("Even better, b > 7");
else
    Print("I don't like your variables");

Here's a version which doesn't precalculate b's goodness until required. This is better when the test is expensive or if it could produce side effects. It also does away with an else, if that really is important. ;-)

var evenBetter = false;
if (a > 5)
{
    Print("Very well, a > 5");
    evenBetter = (b > 7);
}

if (evenBetter)
    Print("Even better, b > 7");
else
    Print("I don't like your variables");

The downside of this "splitting out" method is that a sloppy reader may assume that the second if statement has nothing to do with a and thus fail to note the unusual case that "don't like your variables" occurs when a is good but b is not.

0

Have you tried something like

string badResponse = "I don't like your variables";
string goodReponse = "Very well, a > 5";
string betterReponse = "Even better b > 7";

(a > 5) ? ((b>7) ? Print(betterReponse) : Print(goodReponse)) : Print(badResponse);
sunny_side_of_life
  • 149
  • 1
  • 3
  • 15
  • If a=6 and b=6 this prints "Very well, a > 5" but OPs code prints "Very well, a > 5", "I don't like your variables". If a=6 and b=8 this prints "Even better b > 7" but OPs code prints "Very well, a > 5", "Even better b > 7" – Chris Drew Mar 25 '14 at 11:59
0

if ((a < 5)||(b < 7)) print ("I don't like your variables.");

Invert the logic, and patch it together. What you're doing is an "and"; so check for invalid cases first and proceed with additional checks below it. This also follows the DRY principle.

Calimar41
  • 222
  • 2
  • 11
0
 if (a > 5 && b > 7)
    {
        Print("Very well, a > 5");
        Print("Even better, b > 7");
    }
    else
    {
        if (a > 5)
        {
            Print("Very well, a > 5");
        }
        Print("I don't like your variables");

    }
Meysam Hit
  • 439
  • 1
  • 4
  • 13
0
string msgFirstPart, msgSecondPart = "I don't like your variables";
if (a > 5) {
  msgFirstPart = "Very well, a > 5\n";
  if (b > 7)
    msgSecondPart = "Even better, b > 7";
}
Print(msgFirstPart + msgSecondPart);
Chris Drew
  • 14,926
  • 3
  • 34
  • 54
  • Is this special case or you always prefer to provide [more fun](http://stackoverflow.com/a/22623319/1686291) and [even more](http://stackoverflow.com/a/22622441/1686291). ;) – Not a bug Mar 25 '14 at 12:49
  • 1
    @KishanSarsechaGajjar Special case. Honestly I don't always answer the same question three times! :) I just like the question and had three unique answers and wanted to see what people think. – Chris Drew Mar 25 '14 at 13:17
0

Well, you can do with with 0 else and 0 if, if you are using C++:

switch ( ( a > 5 ) + ( ( b > 7 ) << 1 ) )
{
case 1:
    printf ( "Very well, a > 5" );
case 0:
case 2:
    printf ( "I don't like your variables" );
    break;
case 3:
    printf ( "Very well, a > 5" );
    printf ( "Even better, b > 7" );
    break;
}

C# version has some stealth if statements in the form of ternary operators: (use System.out.print for Java)

switch ( ( a > 5 ? 1 : 0 ) + ( b > 7 ? 2 : 0 ) )
{
case 1:
    Console.WriteLine ( "Very well, a > 5" );
    Console.WriteLine ( "I don't like your variables" );
    break;
case 0:
case 2:
    Console.WriteLine ( "I don't like your variables" );
    break;
case 3:
    Console.WriteLine ( "Very well, a > 5" );
    Console.WriteLine ( "Even better, b > 7" );
    break;
}
KevinZ
  • 3,036
  • 1
  • 18
  • 26
-1

use ? : operator, else will not be required. So that same code will be eliminated and also there will not be another method or goto in picture.

Note : However this will not print exact result if a > 5 and b < 7. Extra "I don't like your variables" will not be printed in that case.

Example :

string result = "I don't like your variables";

if (a > 5)
{
    result = (b > 7) ? "Even better, b > 7" : "Very well, a > 5";
}

Print(result);
Not a bug
  • 4,286
  • 2
  • 40
  • 80
  • For a=6, b=6 and a=6, b=8 this does not print the same output as OPs code. I expect there is a clever way that uses the ternary operator though. – Chris Drew Mar 25 '14 at 08:18
  • This is a way to eliminate `else` and OP wants to eliminate the same code...so both issues are solved in this answer...and OP also don't want to create another method...!!! I am wondering that you properly read question or not... – Not a bug Mar 25 '14 at 08:52
  • if a=6, b=6 then your code outputs "Very well, a > 5" but OPs code outputs "Very well, a > 5", "I don't like your variables". If a=6, b=8 your code outputs "Even better, b > 7" but OPs code outputs "Very well, a > 5", "Even better, b > 7". – Chris Drew Mar 25 '14 at 08:56
  • Ans updated for 2nd case, and I am suggesting just an alternative for eliminating `else`. – Not a bug Mar 25 '14 at 09:06
  • OP's post have two Prints in the first case. Your answer only prints once. – cederlof Mar 25 '14 at 13:53
  • @cederlof You need to improve your reading habit...You didnt event read answer correctly as like as question...[I mentioned that in my answer please read it again.](http://stackoverflow.com/a/22628588/1686291). – Not a bug Mar 25 '14 at 14:05