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?

  • one may argue that conceptually, this has been addressed in How would you know if you've written readable and easily maintainable code? 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 4 '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 4 '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 4 '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 4 '15 at 17:49
  • 1
    @jk Get that functional programming nonsense out of here, blatant state mutation is the only reasonable solution ;) – mortalapeman Mar 4 '15 at 19:12
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.

  • 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 5 '15 at 9:08
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.

  • 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. – user40980 Mar 4 '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 4 '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 4 '15 at 18:13
  • 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 4 '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 4 '15 at 18:57
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; }
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.)

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.

  • 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 4 '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 4 '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.

  • Those places and languages are wrong. – DeadMG Mar 4 '15 at 22:53
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 4 '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 4 '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 4 '15 at 21:00

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.