2 ms·
I will add, this bug occurred essentially because of the following logic bug; 01 if (!A || !B) { 02 if (!B) { 03 fix(B); 04 return 1; 05
by zaroth 7y ago
I will add, this bug occurred essentially because of the following logic bug;
01 if (!A || !B) {
02 if (!B) {
03 fix(B);
04 return 1;
05 }
06 return 0;
07 }
08 return 1;
It’s interesting to think about the IF on Line 02.
It could have read “if (A)” and that would have been correct, but IMO even better to write out “if (A && !B)” in case anything changes with the surrounding code later.
The problem of course is that “A” and “B” are expensive functions which you don’t want to call twice, and it just is so annoyingly verbose to have to assign booleans and then compare those...
You almost want to be able to write out a truth table based on calling the functions and then handling their return values. A kind of 2D switch statement;
switch (A, B) {
case 1,1:
return 1;
case 1,0:
fix(B);
return 1;
default:
return 0;
}
Does any language support a multi-dimensional switch statement like this?
Hmm, maybe it’s not really any better that way either. The case statements are just not explicit enough.
- zenhack 7y agoRe: the language feature: This is standard in ML-family/typed functional languages (OCaml, Haskell...), and lately has been being adopted by a lot of newer languages, including Rust and Swift.
- gpm 7y agoI feel like the "canonical" example of using pattern matching for nested if statements is fizzbuzz fn main() { for i in 1..102 { match (i % 3 == 0, i % 5 == 0) { (true, true) => println!("FizzBuzz"), (true, false) => println!("Fizz"), (false, true) => println!("Buzz"), (false, false) => println!("{}", i) } } } Not only is it obviously correct, but if you somehow manage to forget a case the compiler will tell you.
- egwynn 7y agoPeople give me funny looks when I tell them that I wish all languages had exhaustively-checked destructured pattern matching, but I really do think it’s one of the best “advanced” language features around!
- jacobush 7y agoCouldn't you do this with an anonymous function in languages which have those? Which would look like: { if A && B return println("FizzBuss"); if A && !B return println("Fizz"); if !A && B return println("Buzz"); if !A && !B return println("{}", i); }
- Hello71 7y agothat just seems to being us back to the problem of not wanting to re-evaluate the functions...
- hermitdev 7y agoPattern matching has been added in C# 8.0. See: https://docs.microsoft.com/en-us/dotnet/csharp/pattern-matching https://docs.microsoft.com/en-us/dotnet/csharp/pattern-match...
- enedil 7y agoThat's an odd example, but you can do it in OCaml: match (a, b) with | (1, 1) -> 1 | (1, 0) -> (fix b); 1 | _ -> 0
- hedora 7y agoIn C: switch (!!a) & ((!!b) < 1) { case 3: return 1; case 1: fix_b(); return 1; default: return 0; } If you rewrite 3 as “A & B”, this turns into feature flags, and is less crazy looking (not sure arithmetic is allowed in case statements though). Better (regardless of language) is flow like this: if (!b) fix_b(); return a && b;
- magicalhippo 7y agoWhile it might be annoying to use local variables, I usually find it's superior when debugging. It allows you to easily inspect the value and modify it before going forwards. It also usually helps to make the code more self-documenting
- zenhack 7y agoIt's definitely easier to deal with when using a debugger. But that brings a question to my mind: Why are there so many debuggers that make inspecting the results of intermediate expressions difficult? I should be able to step through my code at a finer granularity than a statement. Re: self-documenting, certainly sometimes (I mean, you shouldn't play code golf...), but in this case the code in question was: 2218 if (!valid_localpart(maddr->user) || 2219 !valid_domainpart(maddr->domain)) { .... 2234 return (0); 2235 } 2236 2237 return (1); It's not obvious to me what you would name the intermediate booleans that would be more self-documenting than the function names used there.
- loeg 7y agoC supports the following, which is short, correct, and easy to understand: save_A = A; save_B = B; if (!A) fix(A); if (!B) fix(B); return (some_expression of save_A and save_B);
- bawolff 7y agoWell the messed up if logic is definitely a cause, i think part of the cause is the pattern of just validating input is safe instead of validating input at ingestion time and escaping just before output (shell out) time. I think doing both is a much more secure pattern* *easy to sit here and armchair make claims of course. I've also only read this post and not the actual code so maybe there is reason for doing it how they did. I'd add for writing the fix domain logic, probably most clear to fix it in one step, and then do the validation as a second step, instead of mixing the two. Minisculey less efficient but its much easier to follow if fixing and validating logic aren't mixed together.
- asveikau 7y agoOr it could pass argv directly to the execve() family instead of relying on the shell to process arguments. (They effectively do exec on sh -c 'cmd') I am going to guess they will be smart enough to redesign this for the next major release, and the current patch is just that, a patch for the old design.
- nitrogen 7y agoYou could do that in C by shifting the bools to make a bitfield.
- beagle3 7y agoAs pointed out in other responses, that's easily down with destructuring and matches in other languages, and even using a regular "case" by building the bits into a number (since the questions have boolean answers in this case). However, as you yourself hinted, the cost (in C anyway) is giving up the short circuit - which might be expensive, or worse - might have unwanted side effects. I'm not aware of a common language construct that would switch/match on both A and B, but defer executing B unless its value is actually needed.
- some1else 7y agoI usually try to flatten the branches, using `else if` when a `switch` statement isn't viable. if (A && B) { return 1 else if (A && !B) { fix(B) return 1 else { return 0 }