Replace counter assignment with breaks#412
Conversation
There was a problem hiding this comment.
I'm happy that this one is equivalent to original code, but whether we want to do this might be a style guide/best practice issue.
I'm not sure whether 'break' always makes things clearer - if there's a lot of nesting of fairly lengthy loops (in terms of code size - which we have some examples of), then it's not always clear at a glance what is being broken, whereas setting the particular variable to break can be more clear, and to me doesn't seem like a terrible thing to do. (Noting that in the second example, which is still very simple, because of the two loops, we have to do both a setting of a loop variable and a break...)
|
In the second example goto would be more idiomatic. In the first it's clear since there is only one loop. I found the current syntax very confusing, as I had to re-check the initial loop conditions to see that it'd end. The way that I proposed it the loop ends directly, instead of changing a variable and then checking its value to end the loop. |
matt-gretton-dann
left a comment
There was a problem hiding this comment.
I would prefer to push #228 over this particular PR.
|
@matt-gretton-dann I'm missing something. In what way is that PR related to this? The lines that I changed here are untouched in it. |
They may have accidentally been referring to my other PR #346 which (once complete) will remove the specified function entirely. |
No description provided.