Should I get rid of continue statements

Question

I've read in this Q&A that using continue statements in loops should be generally avoided. Is the rule worth sticking to for the code below? If yes, what would be the best way to refactor it to get rid of them?

    for (property in formInput) {
        if (!formInput.hasOwnProperty(property) || property === "Id") {
            continue;
        }
        if (property.slice(-3) === "_Id") {
            setMagicSuggestFromFormInput(property);
            continue;
        }
        if (property.slice(-3) === "_bl" && formInput[property] === true) {
            $("#" + property).prop("checked", true);
            continue;
        }
        $("#" + property).val(formInput[property]);
    }

Edit: If you think the loop has to be refactored, besides indicating on how it can be done, could you please tell me why you consider the proposed refactoring a better design choice?

Personal choice ... frankly, I'd rather see `continue` statements that deeply-nested code. — Jeremy J Starcher, Feb 15 '16 at 00:12
Possible duplicate of [Why are "continue" statements bad in JavaScript?](http://stackoverflow.com/questions/11728757/why-are-continue-statements-bad-in-javascript) — kamilk, Feb 15 '16 at 00:22
This question is inherently opinion-based. Asking for best practices or implicit rules will only spawn very opinionated answers lacking technical objectivity. This question is likely to be closed. For more information, see the [help/on-topic]. Please also note that trying to get a "better design choice" is both extremely broad and also opinion-based. — Kyll, Feb 15 '16 at 00:26
I think the first continue is best left as is, as it specifies cases that require no action at all. The rest... I'm not sure. I generally agree that continue is nicer than deeply-nested code, though. — celticminstrel, Feb 15 '16 at 00:27
@Jeremy that's exactly my feeling about it, but for reasons [mentioned here] (http://stackoverflow.com/a/11730237/5130839) you are supposed to avoid continues. I don't understand why refactoring continues would be a nightmare. — chrumck, Feb 15 '16 at 00:30
The answers to the question you linked to already explain that it's very subjective. To me it's a detail that hardly matters. Some developers are very sensitive to code which doesn't look precisely how they would have typed it — kamilk, Feb 15 '16 at 00:30
@kamilk I guess you are right. This piece of code got criticized at code review and I wanted to better understand the reasoning. Maybe I should just say "leave me alone, it's good enough" — chrumck, Feb 15 '16 at 00:41

score 0 · Answer 1 · answered Feb 15 '16 at 00:08

0

Why don't you try "else if"s?

Putting

$("#" + property).val(formInput[property]);

inside an else statement ensures that it won't run when one of the else if statements are run. You'd make the rest of the if statements else if statement.

answered Feb 15 '16 at 00:08

Davidthefat

63
1
8

Yes, I considered using multiple else if, but as mentioned in the comment to the other answer, I don't see the design reason to do that, What would be the benefits of doing that? – chrumck Feb 15 '16 at 00:18

score 0 · Answer 2 · edited Apr 12 '17 at 07:32

When you use break/continue in a loop or block at the beginning they act as a precondition which is a good design and helps understand the block easily. If you use continue or break at the middle of the block with some code around then it acts like a hidden trap and difficult to understand which is bad.

You can check this link for more discussion around break/continue https://softwareengineering.stackexchange.com/questions/58237/are-break-and-continue-bad-programming-practices

you could try something like this with a nested if else block instead.

for (property in formInput) {
  if (formInput.hasOwnProperty(property) && property !== "Id") {
    var propVal = property.slice(-3);
    if (propVal === "_Id") {
      setMagicSuggestFromFormInput(property);
    } else if (propVal === "_bl" && formInput[property]) {
      $("#" + property).prop("checked", true);
    } else {
      $("#" + property).val(formInput[property]);
    }
  }
}

score 0 · Answer 3 · answered Feb 15 '16 at 00:12

If don't want to use continue statements then you can go the route of nested code:

for (property in formInput) {
    if (formInput.hasOwnProperty(property) && property !== "Id") {
        if (property.slice(-3) === "_Id") {
            setMagicSuggestFromFormInput(property);
        }
        else if (property.slice(-3) === "_bl" && formInput[property] === true) {
            $("#" + property).prop("checked", true);
        }
        else{
           $("#" + property).val(formInput[property]);
        }
    }
}

I really like the answers to your linked Q&A though. It's a tool and you should use it when appropriate.

score 0 · Answer 4 · answered Feb 15 '16 at 00:12

0

You could nest the conditional checks and invert the conditions.

for (property in formInput) {
    if (formInput.hasOwnProperty(property) && !property === "Id")
        if (property.slice(-3) === "_Id") {
            setMagicSuggestFromFormInput(property);
        else if (property.slice(-3) === "_bl" && formInput[property] === true)
            $("#" + property).prop("checked", true);
        else
            $("#" + property).val(formInput[property]);
    }
}

answered Feb 15 '16 at 00:12

Sina Madani

1,246
3
15
27

Ok, but it seems unclear to me why adding two levels of nesting is any better than continue. What would be the actual goal of adding this complexity for the sake of removing "continue"? – chrumck Feb 15 '16 at 00:15
It makes it easier to maintain and add to later on. It's easier to see the intention and read when using if-else. Functionality-wise there's no difference I guess. – Sina Madani Feb 15 '16 at 00:18

score 0 · Accepted Answer · answered Feb 15 '16 at 00:28

Your code:

for (property in formInput) {
    if (!formInput.hasOwnProperty(property) || property === "Id") {
        continue;
    }
    if (property.slice(-3) === "_Id") {
        setMagicSuggestFromFormInput(property);
        continue;
    }
    if (property.slice(-3) === "_bl" && formInput[property] === true) {
        $("#" + property).prop("checked", true);
        continue;
    }
    $("#" + property).val(formInput[property]);
}

Is the equivalent of:

for (property in formInput) {
    if (property.slice(-3) === "_Id") {
        setMagicSuggestFromFormInput(property);
    } else if (property.slice(-3) === "_bl" && formInput[property] === true) {
        $("#" + property).prop("checked", true);
    } else if (formInput.hasOwnProperty(property) && property !== "Id") {
        $("#" + property).val(formInput[property]);
    }
}

Which would be the preferred method as it's properly utilizing if/then/else and is easier to reason with logically.

Ok, I guess it goes along [with this post why else if is good](http://stackoverflow.com/questions/9169249/why-we-use-if-else-if-instead-of-multiple-if-block-if-the-body-is-a-return-stat) — chrumck, Feb 15 '16 at 00:55

Should I get rid of continue statements

5 Answers5