Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Better way to structure/new keyword

Tags:

java

c++

Some time ago I came across the following construct which I have rarely seen since, though I use it relatively frequently. I use it typically when checking on a whole list of conditions are true and it prevents large levels of indentation. Essentially it uses a for loop to provide a kind of structured goto. My question is firstly whether there is better way to structure this, secondly whether people like it and thirdly whether a new keyword in java/c++ etc. such as unit { } which would only cause breaks to exit to the end of the unit would be useful and clearer.

ps I realise that it is on slip away from an infinite loop, but I think my paranoia about that has meant its never happened.

Edit: I have added some setup code for the further conditions to try to illuminate problems with chained if then elses

boolean valid = false;

// this loop never loops
for (;;)
{
    if (!condition1)
        break;

    condition2.setup();

    if (!condition2)
        break;

    condition3.setup();

    if (!condition3)
        break;

    valid = true;
    break;
}

if (valid) dosomething();

EDIT:

I have just discovered that in fact there is a way to structure this in java without misusing loops etc. and wondered whether this would similarily be frowned on, though I guess I have missed the boat on this one.

The restructured code looks like this.

boolean valid = false;

breakout:
{
    if (!condition1)
        break breakout;

    condition2.setup();

    if (!condition2)
        break breakout;

    condition3.setup();

    if (!condition3)
        break breakout;

    valid = true;
}

if (valid) dosomething();

Now that removes the misuse of the for loop which caused a lot of the complaints, and is actually a solution I think is quite neat and is what I was looking to find originally. I am guessing that this structure is probably not well known since no one mentioned it, people object to this as strongly?

like image 585
aronp Avatar asked Nov 27 '22 01:11

aronp


2 Answers

The loop is counter-intuitive and would be questioned at code review: "Why do you need a loop if you always break on the first iteration?"

Why not use this?

boolean valid = true;

if (!condition1)
    valid = false;

else if (!condition2)
    valid = false;

else if (!condition3)
    valid = false;

if (valid) dosomething();
like image 182
Steve Townsend Avatar answered Dec 04 '22 20:12

Steve Townsend


You may have heard of these things modern programming languages have, called functions ;) One of the key reasons goto is no longer used is that we can now factor code out into separate functions, and call them instead.

One way to solve your problem would be to put the code in a separate function instead, and return instead of breaking from your pseudo-loop:

void safedosomething() {
    if (!condition1)
        return;

    condition2.setup();

    if (!condition2)
        return;

    condition3.setup();

    if (!condition3)
        return;

    dosomething();
}

or write helper functions (such as bool checkcondition1() { condition1.setup(); return condition1; }) which set up and then test the conditions, and use a boolean flag:

bool valid = true;

if (!checkcondition1())
    valid = false;

if (!checkcondition2())
    valid = false;

if (!checkcondition3())
    valid = false;

if (!checkcondition4())
    valid = false;

if (valid) dosomething();

or a bit more concisely:

bool valid = true;

valid &&= checkcondition1();
valid &&= checkcondition2();
valid &&= checkcondition3();
valid &&= checkcondition4();

if (valid) dosomething();

or just

if (checkcondition1()
  && checkcondition2()
  && checkcondition3()
  && checkcondition4())
    dosomething();

There are plenty of ways to express this, without counterintuitive loops-that-don't-loop.

like image 45
jalf Avatar answered Dec 04 '22 21:12

jalf