0

I'm discussing this with a work colleague. Say we want to sum the numbers from 0 to 9 skipping 5. He prefers this:

int sum = 0;
for(int i = 0; i < 10; ++i)
{
    if(i == 5)
    {
        continue;
    }

    sum += i;
}

I prefer this:

int sum = 0;
for(int i = 0; i < 10; ++i)
{
    if(i != 5)
    {
        sum += i;
    }
}

Any reason to prefer one over the other? Cyclomatic complexity is the same in both cases, right?

FerranMG
  • 11
  • 3
  • one may argue that conceptually, this has been addressed in [How would you know if you've written readable and easily maintainable code?](http://programmers.stackexchange.com/a/141010/31260) If your peers keep complaining about your way of doing things, be it one way or another, you better change to make them feel better – gnat Mar 04 '15 at 17:04
  • 1
    6 of one, half a dozen of the other. Both achieve the same result, both are easy to read and identify what's going on. It's simply a matter of style at this point. Neither is preferred over the other. – Joel Etherton Mar 04 '15 at 17:21
  • Go with whichever is easiest to prove correct. If you don't have to worry about proving correctness, the the choice is subjective, so go with whichever people complain about the least. You're not going to find a difference in trivial examples because they're trivial. – Doval Mar 04 '15 at 17:49
  • 1
    not sure if this would be acceptable as an answer but id actually replace the whole for loop with a filter on a stream – jk. Mar 04 '15 at 17:49
  • 1
    @jk Get that functional programming nonsense out of here, blatant state mutation is the only reasonable solution ;) – mortalapeman Mar 04 '15 at 19:12

7 Answers7

5

I personally would go with the first example, given your task. The first example more closely matches your original problem statement. The 2nd requires a little bit more mental parsing to determine if it fulfills your problem statement.

Look at how you worded what is going on: "Sum the numbers from 0 to 9 skipping 5". That is exactly what the first statement does. The 2nd doesn't read quite the same way.

Ultimately, what is right is going to depend on the context of the greater problem you are going to solve. The less thinking you have to do to verify it's doing what it is supposed to, the better.

whatsisname
  • 27,463
  • 14
  • 73
  • 93
  • 1
    There are certain philosophies of programmers that abhor the use of `break`, `continue` or similar loop control structures outside of `if` excluding blocks. Not that this is a good or bad thing (I come from a perl `next LOOP unless ...` tradition)... just that seem to avoid those control structures whenever possible. –  Mar 04 '15 at 17:44
  • 1
    Saying code should look like your requirements is a dangerous trap. It's essentially saying that you should let the business people write the code. – Telastyn Mar 04 '15 at 18:10
  • 1
    @Telastyn: I didn't make such a general statement. How does "make things easy for yourself" equate to that? – whatsisname Mar 04 '15 at 18:13
  • "The first example more closely matches your original problem statement." - in business the original problem statement is written by product owners or business analysts. It might be easier to translate requirements literally, but it's usually not better. – Telastyn Mar 04 '15 at 18:28
  • 1
    @Telastyn: the original problem statement isn't necessarily a requirement from any sort of specification. You are making many assumptions which aren't necessarily true. – whatsisname Mar 04 '15 at 18:54
  • 1
    I'm pretty sure that I'm not the only one reading "original problem statement" as "the issue/requirements in the ticket sent by that business person". If that's not what you mean, maybe you should clarify. – Telastyn Mar 04 '15 at 18:57
4

As it stands right now (a single condition) I don't see much reason to prefer one over the other, but I'd probably prefer the second as slightly simpler.

When, however, your set of conditions are more complex, some variant of the first can be preferable, often by a wide margin. Consider something like:

for (...) {
    if (i != 5 && (i%2!=0) && i>99 && i<1000 && (i%17 !=3))
        sum += i;     
}

vs:

for (...) {
    if (i==5)
        continue;
    if (i%2 == 0)
        continue;
    if (i<100 || i > 999)
        continue;
    if (i % 17 == 3)
        continue;
    process(i);
}

In this case, the latter seems (to me) more understandable and maintainable. It lets us group the items that are logically connected (the range, 100...1000) together, but keep those that aren't so closely related separate so each can be analyzed on its own more easily. It also fits closely with a pattern I use in quite a few functions that basically works out to something like:

function(x, y, z) { 
    if (!preconditions(x, y, z))
        // return an error or (preferably) throw an exception

    // now do processing on x, y, z
}

Although in your case it's a loop body instead of a function per se, you still end up with the same basic pattern: assure that some set of preconditions is met, and then if (and only if) it s, process the data.

Jerry Coffin
  • 44,385
  • 5
  • 89
  • 162
  • Good point here. For one single condition, I find not using the continue easier to read. For a set of conditions or preconditions, maybe discarding the ones that don't apply easier. Thanks for your input! – FerranMG Mar 05 '15 at 09:08
2

My suggestion would be to extract that logic out into a method so that if you add more conditions or even change your condition it is easier to maintain in one place + it is more testable.

The reason I am suggesting that is because I am assuming this is not the actual problem but it is a more simplified version of your problem that you have narrowed it down to demo it here.

If, however, my assumptions are wrong then it would still be a good idea to extract it out because it makes it a lot more readable and understandable (mostly because of the method name and the variable names) - so that when you come back to it after a while it is still easy to grasp and know what you meant when you wrote it the first time.

    private void sumItUp()
    {
        int sum = 0;

        for (int numberToAddToSum = 0, i = 0; i < 10; numberToAddToSum = SkipIfNextOneIsFive(i), i++)
        {
            sum += numberToAddToSum;
        }
    }

    private int SkipIfNextOneIsFive(int i) { return i + 1 == 5 ? 0 : ++i; }
AvetisCodes
  • 1,544
  • 3
  • 14
  • 26
1

Both of those code fragments are doing way too much work. You can replace them with int sum = 40; (in fact, if you gave those code fragments to a compiler, that's basically what the compiler would do). There's a famous trick for summing up the first n integers I used to do it in my head, but if you didn't know the trick and had no calculator handy, you could work it out with pen and paper in about the same amount of time it would take to type up that code. So the problem, as stated, isn't something you should be writing a program for at all.

Of course, you aren't asking about only that one example problem. What problem are you actually trying to solve? The question overspecifies a bit, so I'll just guess that you're either (a) going for something that sums up all the consecutive integers over a range except for a specified one, or (b) you're going for summing up all the consecutive integers in a range, except for a range in the middle.

If you want (a), then you can just subtract the specified number from the range. If you want (b), then you can break the large range into a bottom half and a top half and deal with each separately. So, in either case, we want something that can sum up a range of integers.

If you know the famous formula for summing up the first n integers (n*(n+1)/2), and you know how it works, it isn't hard to come up with a formula for a range ((top-bottom+1)*(top+bottom)/2). If not, there's the obvious and straightforward way of doing it with for loops.

If we write a function that does that (with either method), we can rewrite the original problem as sum_range(0,9) - 5 or sum_range(0,4) + sum_range(6,9). It turns out we didn't need any ifs or continues, and, depending on how we wrote it, maybe no for loop either.

If your question is really about which keyword to use when nested inside a construct using a specific keyword which is nested inside another construct using another specific keyword, then I don't think your question really has an answer, and that trying to figure out how to program by looking at which keywords nest inside which other keywords is never going to be useful.

(For those interested, the formula works like this: say we're trying to add up all the integers from 1 to 100. 1+100 = 101, 2+99 = 101, 3+98 = 101, and so on, so we could just multiply 101 by the number of pairs, and the number of pairs is the number of numbers being added / 2. In this case, top = 100, bottom = 1, top+bottom = 101, and the number of pairs is (top-bottom+1)/2 = n/2 = 50.)

Michael Shaw
  • 5,116
  • 1
  • 21
  • 27
0

Technically he will have one more line of execution for the continue statement. Otherwise, the code is identical. It is O(n) in both cases.

Lawrence Aiello
  • 1,398
  • 11
  • 12
  • 1
    That's not necessarily true, as they could easily compile to the same thing with optimizations enabled. Many optimizing compilers can even replace the whole loop with sum = 40 and skip the whole thing. – whatsisname Mar 04 '15 at 18:08
  • But that has nothing to do with the runtime analysis of the code. That's a compiler trick and has no baring in algorithmic analysis – Christian Bongiorno Mar 04 '15 at 21:02
0

Any reason to prefer one over the other?

At some places and in some languages, the use of continue and break are frowned upon as glorified gotos. They make it harder to read the code since you need to stop and then find the end of the loop you're in rather than noting the condition and reading along.

The second version will work in pretty much any language too. continue isn't always available.

Telastyn
  • 108,850
  • 29
  • 239
  • 365
0

A for loops natural tendency is to "continue" and as others have pointed out, it's an extra line of code.

In 15 years of writing code I have MAYBE used continue (in any language) 3 times; and none of that was recent. It's simply not a necessary construct as you quickly realize.

  • 1
    And I've used continue roughly a gazillion times. Different kinds of software will use different things. – whatsisname Mar 04 '15 at 18:11
  • No, different kinds of PROGRAMMERS will use different things. A lot depends on what you grew up with. PERSONALLY, I do not use "continue", and I try very hard not to use "break" in anything other than a C/C++ "switch" statement "case" arm. – John R. Strohm Mar 04 '15 at 18:49
  • Using 'continue' is the logical equivalent of 'if the loop should continue then let it continue' But, the built in predicate of the loop is always going to be evaluated and will continue on true. It therefore stands to reason that your 'continue' statement should either be in the predicate of the loop in some fashion or not exist at all. As for 'style'? Sure, you an use it for 'style' purposes. But evidence by having not written them or even seen them in a decade (in any code base, C/C++ or Java) it stands as clear evidence of being unnecessary. As is, for example, breaking to labels in java – Christian Bongiorno Mar 04 '15 at 21:00