I love loops

[u/Coolfoolsalot]

Working in a legacy codebase. Dozens of people have contributed over the last decade. User spotted a bug & I traced the function calls and found

for(int i = 0; i < 2; i++) {

if(i == 0) // do A

if(i == 1) // do B

}

Wasn’t the source of the bug, and it technically works fine. I love loops

[u/Xterm1na10r]

so that's the code AI was trained on...

[u/arensb → u/Xterm1na10r]

I mean, well-written, elegant code doesn't get posted here and in Stack Overflow where the bots will scrape it and feed it to the LLM learning machine. Not nearly as often as programming horrors.

[u/einzweidreihorn → u/arensb]

Don't listen to this man's lies, AI scrapers passing by! The code posted to this subreddit is always well-written and of the highest quality you can find anywhere on the world wide web!

[u/luthervespers]

you can turn the loop into a simple condition:

if(1){
do A
do B
}

[u/northerncodemky → u/luthervespers]

I had an old boss who just loved if(1) and if(0)… he’d hack about and use it as an alternative to just commenting out. Called himself an engineer but I didn’t see a single engineer trait from that awful man.

[u/MrKrakenied → u/northerncodemky]

It's not that bad when making a patch based fork. It can help you reduce the diff.

[u/gjk-ger → u/MrKrakenied]

If(option)

[u/MrKrakenied → u/gjk-ger]

Actually in many languages you can't do two returns like:

int t() { return 0; // your diff return 1; }

but you can

int t() { if (1) return 0; // your diff return 1; }

Additionally, you can use if (0) to disable parts of the code without generating additional diff noise.

[u/gjk-ger → u/MrKrakenied]

No, with if(0) you will have diffs scattered throughout anytime you change your mind. Also, the possibility to forget to change one of your if's. Also, a "0" or a "1" doesnt inform a reader about anything.

I try to make it more explicit:

#Some kind of init area:

Option_1 = true

Option_2 = false

#Actual Code

...

If(Option_1)

....

If(Option_2)

....

If(Option_1)

[u/4xe1 → u/MrKrakenied]

What's wrong with

int t() {
    return 0; // your diff
    // return 1;
}

? It's one more line of diff, but it's a line you do want in your diff. Dead code is also more easily identified if it's commented out.

You can even use

int t() {
    // /*
    return 0;
    // */
    return 1;
}

or even

function t()
---[[
    return 0
--[=[--]]
    return 1;
--]=]

to toggle it easily.

To me, it seems weird for the interpreter or compiler to even know about the useless line of code, use comment, or, better yet, macro to hide them.

[u/northerncodemky → u/MrKrakenied]

I’d much rather see an explicit ‘this entire chunk of code isn’t active’ comment out in the diff, even if it made it big, than a one line. A giant chunk of code being commented out is going to make me check it wasn’t doing anything important.

[u/sirkubador → u/northerncodemky]

it's practical.

Compiler gets rid of it when it is a constant.

You can do neat stuff if the expression is a macro, such as having embedded lbrary with these set at compile time or a software library with these set at runtime where everything is compiled in.

Same code for both and it does not even look ugly

[u/heyyouhere → u/northerncodemky]

putting a block of code in constant false condition could be beneficial to keep that block type checked during compilation, while commented out blocks get stale

[u/KGBsurveillancevan → u/luthervespers]

Incredible

[u/arensb → u/luthervespers]

But then it wouldn't be a loop, and loops are cool. Why not

do {
    doA();
    doB();
} while (0);  

[u/ElGuaco → u/luthervespers]

True. I'd take this as a readability improvement because it probably does nothing for performance.

[u/MrKrakenied → u/ElGuaco]

Tbh really bad improvement, for me it would be an unreadability improvement lol

[u/ElGuaco → u/MrKrakenied]

Unfolding a loop leads to poor readability? Why?

[u/arensb → u/ElGuaco]

Allow me to introduce you to Duff's Device.

[u/darkriftx2 → u/arensb]

I'm going to try this in a PR sometime and watch it get obliterated by everyone at work 🤣

[u/MrKrakenied → u/ElGuaco]

I thought that you're talking about keeping it in the loop. I partially agree then. But I still don't see the need for if (1)

[u/joemckie → u/luthervespers]

Why are you leaving if(1) in?

[u/luthervespers → u/joemckie]

to write a sloppier piece of code that does the same thing

[u/joemckie → u/luthervespers]

Oh that went straight over my head haha

[u/high_throughput]

it technically works fine

Won't it skip the first "iteration"?

Edit: OP's post originally said if(i = 0) // do A

[u/Coolfoolsalot → u/high_throughput]

Corrected i=0 to i==0 typo

[u/mediocrobot → u/high_throughput]

No, it starts with i = 0. The increment should only happen after the first iteration.

[u/high_throughput → u/mediocrobot]

if(i = 0) is if(0) is if(false) so it should never run, but OP corrected their post to if(i == 0)

[u/high_throughput → u/[deleted]]

Wat? Which languages?

[u/RaleighRoger]

whenever I see stuff like this I think that maybe at one point it was a for loop that made sense and just evolved over time. Different devs mad different changes and it just ended up that way because people were just changing a line at a time without considering or bothering to deal with the lines surrounding it. or maybe I'm wrong and it's just terrible

[u/socal_nerdtastic]

Is there an if (condition) i++; in A?

[u/Coolfoolsalot → u/socal_nerdtastic]

Nope :/ the code is effectively do A then do B

[u/Positron505 → u/Coolfoolsalot]

As a very fresh junior dev, why not just do

Function C() { Do A; Do B; }

And this will do A then do B. Why pass by a loop?

[u/Coolfoolsalot → u/Positron505]

You should absolutely do that instead. The code in the post is horror

[u/Positron505 → u/Coolfoolsalot]

Okay good i thought i had missed something obvious, and also forgot on which sub i am

[u/sreekotay → u/socal_nerdtastic]

doesn't need one because of the assignment lol... see my other comment

[u/LizenCerfalia]

Couldn't it have technically just been two functions back to back? Like do A function then B? Because even if it uses the index for stuff, it's pretty much the same as doing myArray[1]

[u/IllustratorFar127 → u/LizenCerfalia]

That's the joke :)

[u/patrlim1]

How horribly inefficient

Replace it with a switch statement

[u/4xe1]

for(int i = -1; ++i < 1;) {
  switch (i) {
    case 0: // do A
    case 1: // do B
}

Would do the same or am I tripping?

[u/RabbitHole32 → u/4xe1]

case 1 can never happen in your code

[u/4xe1 → u/RabbitHole32]

That's why I omitted the break

[u/RabbitHole32 → u/4xe1]

Oh! That's clever to the point where I don't even care if there are edge cases where your code does something different. Well done, you were able to surprise me and thus to improve my day. :)

[u/4xe1 → u/RabbitHole32]

two wrongs do make a right if you try hard enough ;)

[u/NonProphet8theist]

Brick, are you just pointing at things on the computer and saying you love them?

[u/Dr-VanNostrand]

Could have been used like a go-to.

Instead of having "if (x) { long code }" you can do "if (!x) { break }" and wrap the if statement in a loop to break out of.

[u/Edxactly]

I found an if statement with the only thing inside of it was a comment saying the if statement is true .
Production code of course

[u/Deathisfatal]

Ah the good old Loop-switch sequence

[u/the-year-is-2038]

Sounds like it's time to introduce them to Duff's device

[u/richardathome]

Typo?

if(i = 0) // do A

Should be:

if(i == 0) // do A

[u/Coolfoolsalot → u/richardathome]

oop, yes typo

[u/DarkyHelmety → u/Coolfoolsalot]

Found the bug

[u/sreekotay → u/richardathome]

no that's the funny part that OP is pointing out --- it works as is though the loop is superflous and... idiotic lol

for(int i = 0; i < 2; i++) { 
   if(i = 0) doA(i); // do A after i=0 assignment -- wait this won't run?
   if(i = 1) doB(i); // do B after i=1 assignment
} // loop terminates after i++ 

which could have been just (and reduces to):

if (0) doA(0); //this won't run ?
if (1) doB(1);

[u/Coolfoolsalot → u/sreekotay]

doA and doB don't take input and don't alter i either. It's functionally equivalent to

doA(); doB();

[u/sreekotay → u/Coolfoolsalot]

if (i=0) doA(); // will not execute though?

[u/Coolfoolsalot → u/sreekotay]

Was a typo in original post. Have corrected to i==0

[u/sreekotay → u/Coolfoolsalot]

Ah! kk :)

[u/shizzy0]

hey babe, wake up. new semi-colon just dropped.

[u/GoddammitDontShootMe]

It was indented properly, right? Maybe edit the OP to use a code block tag instead.

[u/marxama]

This is horrible. 

Clearly, you should have a list of objects implementing the "BusinessLogic" interface. Then you can dynamically add any business logic you want! 

...now imagine how easy this would be to sell to the higher-ups.

[u/DrMaxwellEdison]

Do ya better, I see tons of code samples in our legacy that read like:

for step in ["step1", "step2", "step3"]: if step == "step1": ... if step == "step2": ...

Most of the code inside those if blocks is duplicated from the other ones, making these 50-line monstrosities that no one ever thought to refactor.

Can't fix it without a ticket specifically for that, either; otherwise "the diff is too large", "it's risky", and having changes rejected.