Code review, here the simplified version. What the fuck has to be wrong with someone who seriously codes the first variant in production code?!

Insanity.
I've seen so many gigantic tangled messes like that before. I had to run some of them through wolfram alpha to simplify. 😅

But please include parentheses around the logic groups. Please. Especially when they're non-trivial

@Root was thinking the same thing. This drives me crazy.

@Root yeah in the real code, they are there, but I simplified them away. :-)

The 2nd one is still not good enough. I always have problems with preference unless enclosed in parentheses

@asgs see my previous comment.

Those are two different things, the expressions evaluate differently

Example:
1) do_thing if A is true, regardless of the values of the rest.
2) if A is true and E is false , it will do_something_else

I suggest to run it through a truth table generator.

@mundo03 1. The second version will also call do_shit() if A is true.
2. If A is not true, and the rest are also false, it will run the do_some_other_shit() part in both versions.

The key to notice is that in the first version, the second "if" will only be entered if A is not true because otherwise, the first "if" would have been executed. This makes the !A automatically true in the second "if".

@Fast-Nop wait.
In version 1 is A is true it do-something.
If A true and E true, it do_something too.

In version 2
If A true and E false, it do_something
_else
Because it will evaluate to true && whatever && false = false
Case 1 ignores B C D E when A is true.
Case 2 takes all inputs, always.

So they are different.

I know you left out some parentheses which might correct this, but how do you want me to know if you don't put them in?
This code review is a no go, do it again and submit again asap.

@mundo03 uhm no. In version 2, it will also do_something and not something_else because in version 2, the if-expression is already true after evaluation A so that the rest doesn't matter.

See, the second version has || after A, and if A is true, then it is TRUE || morestuff, which is always true no matter what morestuff is.

Please double-check: there is no && after A in the second version, it's ||.

@Fast-Nop I was thinking && preceded || in operation order, but it seems there is no such thing.

😁

@mundo03 well && does have a higher binding than ||, that's the operator precedence.

Anyway, I've made an automated check, and both variants are the same - no "Disagree" output.

@jschmold ah that's just my habit to silence compiler warnings for intentionally unused variables.

Also quite useful if you want to read out a volatile variable, but don't need the value itself because the act of reading resets some flag. That's common with status registers.

@jschmold that's wired into the silicon. Last I had that with the systick overflow status bit on a Cortex-M4. You reset this bit by reading the control and status register where this bit is in.

And volatile is necessary because otherwise, the compiler would optimise the read operation away since is has no visible effect. With volatile, the compiler has to actually do the read and then discard the value.

You won't understand. This is art.

@maushax that was just the tip of the iceberg. I didn't want to recreate everything, and a literal screenshot would violate my work contract.

But the do_shit() stuff was actually not a function call, but another if-statement with a big switch-case inside, and it was copy/pasted with just one constant changed. Instead of setting a variable and doing the switch-case once. So basically the same switch-case copy/pasted 4 times.

The whole function was over 200 lines, out of which only 80 were needed.

And I've been ticked off over that specific codebase anyway after I had seen the 3500 lines function with a loop that contains a 2000 lines switch-case, and that contains other switch-cases of around a 100 lines or so.

The guy who did that was really a shithead who even had dared to tell off other colleagues, among them one whose skills I highly appreciate after I inherited his codebase (another one).

@maushax well he would, but he left the company before I entered. He thought that he was a super pro when he actually didn't know jack shit. Our clash would have been epic. ^^

I have seen worse.. I asked one of my team mates to add a check if its a number before parsing it to float and doing some calculations in JS ..

This is the validation he wrote..

if((typeof bal == 'number' || bal && typeof bal == 'object' && toString.call(bal ) == numberClass || false) && (new RegExp(""^[0-9]"").test(bal)))

Fun note : 'numberClass' wasn't even defined..

When i asked him to break it down and explain to me what each condition did ..he had no fucking idea..

@asgs parentheses should indeed be added, even more because in terms of logic the expression may not evaluate as intended without parentheses.

@CodeMasterAlex for enhanced readability yes, but the term evaluates exactly as intended and specified by the C standard, short-circuit evaluation included.