3 ms·

Oof really? That’s a horrible pattern! In my professional opinion of course. Separation of causes, man! :)

porridgeraisin 3y ago
It's very common in C while((buf = resultofstreamingoperation() != null) { } The ` != null ` is redundant here but you can imagine where it'd be required.
- sweetjuly 3y ago
  A more common form (and slightly less evil) is for error checking: if ((error = do_a_thing())) { // some error happened, good thing we saved the error code } Given the standard "zero is success" paradigm, handling errors is very succinct.
- demondemidi 3y ago
  You are arguing "common" == "good". That's not true.
kmoser 3y ago
Yeah, but separation of causes then violates the DRY principle because you have to repeat the 'a' expression twice: a = b if ( a == c ) ... The repetition is even worse if you replace 'a' or 'b' with a more complex expression: a[foo/bar] = b[baz/bat] if (a[foo/bar] == c) ... Oof indeed! Just rewrite it as: if ( ( a[foo/bar] = b[baz/bat] ) == c ) ... // Yes, assign and test!
- iamflimflam1 3y ago
  Wow - I’ve been coding for a long time and find your assign and test really hard to parse. In a code review I would definitely reject this.
  - kmoser 3y ago
    Just curious, do you also find the ternary operator hard to parse? This is a serious question: I've found some programmers tend to avoid it entirely, while I think there are some clear cases where it's advantageous to use. What I'm getting at is one person's "easy to parse" is another person's "difficult to parse", and there may be no objective answer which makes one any better than the other.
    - iamflimflam1 3y ago
      To be honest I think it really depends. And in some contexts it is unavoidable. I would think to myself if I was writing one “have I made a mistake somewhere that means I have to use this?” I don’t think it’s necessarily about “can I parse this”, it’s much more about will all the devs on the team be able to parse this. To me, code is all about communication - to myself, but also to an audience I haven’t met with varying skill, knowledge and context levels. I’ve been bitten too many times with code that only one or two people can work on.
- nickthesick 3y ago
  I hope this is satire, DRY is more about extraction of shared function not about individual expressions. You write code once, no need to save yourself typing time at the expense of you and other peoples ability to read and understand the code afterward.
  - kmoser 3y ago
    > DRY is more about extraction of shared function not about individual expressions I'm not sure where you get that idea. Repetition anywhere is usually a code smell. > You write code once And then you update it when you add new features or the requirements change or you fix bugs, etc. Having to change two symbols is more error prone than changing one. And having to parse more code is harder than parsing less code. The assign-and-test pattern is common in several languages (e.g. C), and adding a comment that explains the logic should remove all doubt as to what is happening and why, so I see it as a win/win. In any case, there is a trade-off between terseness and legibility, and while I usually favor more verbose code, I tend to draw the line at needless repetition. But that's my personal preference, and everybody draws that line in different places.
    - rini17 3y ago
      > And having to parse more code is harder than parsing less code. I'd rather parse three straightforward copies with minor differences than one chunk of dry spaghetti. And in assign & test case, splitting them makes debugging much easier.
- kawhah 3y ago
  Hmm, no. You rewrite it as key1 = foo/bar key2 = baz/bat a[key1] = b[key2] if (a[key1] == c) I'm sure a coder who understands the context can come up with better names than key1 and key2. edit: or better yet key2 = baz/bat normal_var = b[key2] if (normal_var == c) ... key1 = foo/bar a[key1] = normal_var
  - kmoser 3y ago
    While your solution splits the whole thing into smaller and arguably more easily digestible steps, you have introduced two new variables, key1 and key2, which need to be carefully scoped (if they aren't already) so they don't clash with existing variables. If your project consists of many instances of code like this, you end up with many variables which can become harder to manage. You're also splitting what is intended to be essentially an atomic operation into multiple steps, which can be good if you want to analyze and tweak them in the future, but it's now no longer clear where the process begins and ends in relation to the code that comes before and after: you have to add more comments, or split the whole thing out into its own function. I'm not saying your code is bad or wrong, just that there are downsides to any solution (including mine), and ultimately everybody has to pick whichever has the fewest negatives for their particular project.
- demondemidi 3y ago
  Mission critical automotive code under ISO(ASIL) that usually follows MISRA, assign and compare in the same clause is banned. If you're writing C code that lives don't depend on, do whatever you want, but I pity people that have to own your code.

It's very common in C while((buf = resultofstreamingoperation() != null) { } The ` != null ` is redundant here but you can imagine where it'd be required.

sweetjuly 3y ago
A more common form (and slightly less evil) is for error checking: if ((error = do_a_thing())) { // some error happened, good thing we saved the error code } Given the standard "zero is success" paradigm, handling errors is very succinct.
demondemidi 3y ago
You are arguing "common" == "good". That's not true.

kmoser 3y ago

Yeah, but separation of causes then violates the DRY principle because you have to repeat the 'a' expression twice: a = b if ( a == c ) ... The repetition is even worse if you replace 'a' or 'b' with a more complex expression: a[foo/bar] = b[baz/bat] if (a[foo/bar] == c) ... Oof indeed! Just rewrite it as: if ( ( a[foo/bar] = b[baz/bat] ) == c ) ... // Yes, assign and test!

iamflimflam1 3y ago
Wow - I’ve been coding for a long time and find your assign and test really hard to parse. In a code review I would definitely reject this.
- kmoser 3y ago
  Just curious, do you also find the ternary operator hard to parse? This is a serious question: I've found some programmers tend to avoid it entirely, while I think there are some clear cases where it's advantageous to use. What I'm getting at is one person's "easy to parse" is another person's "difficult to parse", and there may be no objective answer which makes one any better than the other.
  - iamflimflam1 3y ago
    To be honest I think it really depends. And in some contexts it is unavoidable. I would think to myself if I was writing one “have I made a mistake somewhere that means I have to use this?” I don’t think it’s necessarily about “can I parse this”, it’s much more about will all the devs on the team be able to parse this. To me, code is all about communication - to myself, but also to an audience I haven’t met with varying skill, knowledge and context levels. I’ve been bitten too many times with code that only one or two people can work on.
nickthesick 3y ago
I hope this is satire, DRY is more about extraction of shared function not about individual expressions. You write code once, no need to save yourself typing time at the expense of you and other peoples ability to read and understand the code afterward.
- kmoser 3y ago
  > DRY is more about extraction of shared function not about individual expressions I'm not sure where you get that idea. Repetition anywhere is usually a code smell. > You write code once And then you update it when you add new features or the requirements change or you fix bugs, etc. Having to change two symbols is more error prone than changing one. And having to parse more code is harder than parsing less code. The assign-and-test pattern is common in several languages (e.g. C), and adding a comment that explains the logic should remove all doubt as to what is happening and why, so I see it as a win/win. In any case, there is a trade-off between terseness and legibility, and while I usually favor more verbose code, I tend to draw the line at needless repetition. But that's my personal preference, and everybody draws that line in different places.
  - rini17 3y ago
    > And having to parse more code is harder than parsing less code. I'd rather parse three straightforward copies with minor differences than one chunk of dry spaghetti. And in assign & test case, splitting them makes debugging much easier.
kawhah 3y ago
Hmm, no. You rewrite it as key1 = foo/bar key2 = baz/bat a[key1] = b[key2] if (a[key1] == c) I'm sure a coder who understands the context can come up with better names than key1 and key2. edit: or better yet key2 = baz/bat normal_var = b[key2] if (normal_var == c) ... key1 = foo/bar a[key1] = normal_var
- kmoser 3y ago
  While your solution splits the whole thing into smaller and arguably more easily digestible steps, you have introduced two new variables, key1 and key2, which need to be carefully scoped (if they aren't already) so they don't clash with existing variables. If your project consists of many instances of code like this, you end up with many variables which can become harder to manage. You're also splitting what is intended to be essentially an atomic operation into multiple steps, which can be good if you want to analyze and tweak them in the future, but it's now no longer clear where the process begins and ends in relation to the code that comes before and after: you have to add more comments, or split the whole thing out into its own function. I'm not saying your code is bad or wrong, just that there are downsides to any solution (including mine), and ultimately everybody has to pick whichever has the fewest negatives for their particular project.
demondemidi 3y ago
Mission critical automotive code under ISO(ASIL) that usually follows MISRA, assign and compare in the same clause is banned. If you're writing C code that lives don't depend on, do whatever you want, but I pity people that have to own your code.