|
*whoosh*
|
# ? Feb 26, 2010 16:40 |
|
|
# ? May 15, 2024 03:23 |
|
FateFree posted:I had to post this. I've seen a lot of lovely code in my day, but this is hands down the worst method I've ever seen written in my life. This simple method shows such a misunderstanding of the basic principles of coding with Java and coding in general. That is loving amazing. Who wrote that, anyway?
|
# ? Feb 26, 2010 19:08 |
|
Holy gently caress I'm the coding horror today. Tracing a data-related bug in a large LAMP application, I've been drumming up lots of mysql queries all day. Then, after coming down to a good candidate record to follow the bug with, I add something to make the logs more useful after a test run, and drop it in at the top of the super class save() function that EVERY piece of data goes through when it saves (ie. an insert/update is built just a few lines down): php:<? if( $this->id='e08180c3-be7d-069c-bbfe-4b735109d8ff') {...} ?> At least it was on a local copy, and i have a recent db snapshot, but goddamnit that was a loving stupid bug to come up with. EDIT: I suppose it was a clever new way of corrupting lots of data with minimal keystrokes! Bhaal fucked around with this message at 22:55 on Feb 26, 2010 |
# ? Feb 26, 2010 22:53 |
|
At work yesterday, I tracked a bug down to this horrible function, which immediately made me think of this thread. You can't see it in the HTML rendered version there, but it mixes hard and soft tabs freely, so the indentation is basically nonsense. Worse than that, it uses a switch statement to decide how to process the strings. The cases have no labels other than comments, so you see "process_case = 3;" and have to go find which case that means. Definitely couldn't have used functions for this! Then I saw this awesome bit: pre:if(...){ ... }else do{ if(...){ ... break; } ... }while(FALSE); /* a little trick to allow easy exit from nested if's */
|
# ? Feb 27, 2010 17:33 |
|
ColdPie posted:At work yesterday, I tracked a bug down to this horrible function, which immediately made me think of this thread. You can't see it in the HTML rendered version there, but it mixes hard and soft tabs freely, so the indentation is basically nonsense. I saw this do{}while(false); pattern quite a bit and apparently it is considered ok as you don't have to use goto and can use break in do and it only executes once.
|
# ? Feb 27, 2010 18:57 |
|
RussianManiac posted:I saw this do{}while(false); pattern quite a bit and apparently it is considered ok as you don't have to use goto and can use break in do and it only executes once. It's been mentioned several times in this thread already. Personally, I think it's a horrible abuse of loop syntax that accomplishes the same thing as a goto, but in a less clear manner.
|
# ? Feb 27, 2010 18:59 |
|
RussianManiac posted:apparently it is considered ok as you don't have to use goto and can use break in do and it only executes once. You have no idea what a break really is, do you?
|
# ? Feb 27, 2010 18:59 |
|
RussianManiac posted:I saw this do{}while(false); pattern quite a bit and apparently it is considered ok as you don't have to use goto and can use break in do and it only executes once. On any system I've worked with that would generate exactly the same code as a goto would except it makes the source look even worse.
|
# ? Feb 27, 2010 19:30 |
|
RussianManiac posted:I saw this do{}while(false); pattern quite a bit and apparently it is considered ok as you don't have to use goto and can use break in do and it only executes once. This is why things like "goto considered harmful" that have gotten themselves inserted into the conventional wisdom are completely stupid. The following two pieces of code are functionally equivalent: code:
code:
Alexander Stepanov used goto in "Elements of Programming" to model a state machine. If it's good enough for him, then it's good enough for me. The lesson here is that every code construct, design pattern, or whatever, has appropriate and inappropriate uses, and the mark of a good programmer is to use them appropriately instead of devising "clever" tricks like the one above. This all could have been avoided if the paper had been titled "Stupid uses of goto, as well as stupid tricks to avoid using goto, considered harmful". Flobbster fucked around with this message at 19:37 on Feb 27, 2010 |
# ? Feb 27, 2010 19:32 |
|
edit: nevermind, completely missed that it was while(FALSE) and not while(TRUE), because if it were while(TRUE) it would have been appropriate to use break over goto
Khorne fucked around with this message at 20:03 on Feb 27, 2010 |
# ? Feb 27, 2010 19:59 |
|
6174 posted:You have no idea what a break really is, do you? And you apparently don't understand the purpose of a syntax more expressive than asm.
|
# ? Feb 27, 2010 20:00 |
|
Flobbster posted:This all could have been avoided if the paper had been titled "Stupid uses of goto, as well as stupid tricks to avoid using goto, considered harmful".
|
# ? Feb 27, 2010 20:00 |
|
Khorne posted:Those aren't the same. You need the goto example to loop in some way for them to be equal, or you need two goto statements. What? No. There's no looping in that example. This: code:
code:
Dijkstracula posted:Of course, if people actually read the paper they'd understand that what was being discussed was BASIC-style GOTOs, not what C has. I think we're on the same page here. I'm not criticizing the content of the paper per se, just those who only read the title and use it to form the basis for all kinds of tricks meant to avoid typing a simple keyword. But I don't need to tell you that, judging by your username
|
# ? Feb 27, 2010 20:05 |
|
Khorne posted:Those aren't the same. You need the goto example to loop in some way for them to be equal, or you need two goto statements. That is where goto gets messy. You end up with all these arbitrary points in the code, and your final goto is always beyond where the person read and with no clear "position" like brackets have. How are they not the same? That do/while loop only executes a single time. There is no actual looping involved, which is why using a loop statement with a break is less clear than just using a goto statement. efb, hard.
|
# ? Feb 27, 2010 20:05 |
|
king_kilr posted:And you apparently don't understand the purpose of a syntax more expressive than asm. In the context of a do {} while (false); a break is being used as a goto, and in reality is an unconditional jump (goto). Using syntax that disguises what you are doing is not more expressive.
|
# ? Feb 27, 2010 20:18 |
|
So there are actually several strong advantages to breaking out of a loop instead of using goto. You don't have to invent a unique-in-the-function label; it's immediately obvious where the jump goes to, so your readers aren't forced to search for the label and hope it's in a sensible place; and most importantly, it guarantees that you can't skip variable initializations (*). These advantages are simply the general advantages of structured programming. JavaScript does this very well. You're still forced to use goto for state machines, though; tail-recursive calls to nested functions are better, but aren't available (or optimized reliably to jumps) in any mainstream imperative languages. (*) C++ restricts this so that you can't goto (or switch) past a non-trivial initialization. C's only restriction is that you can't skip a VLA declaration.
|
# ? Feb 27, 2010 22:18 |
|
which is worse:code:
code:
|
# ? Feb 27, 2010 22:54 |
|
rjmccall posted:So there are actually several strong advantages to breaking out of a loop instead of using goto. You don't have to invent a unique-in-the-function label; quote:it's immediately obvious where the jump goes to, so your readers aren't forced to search for the label and hope it's in a sensible place; quote:and most importantly, it guarantees that you can't skip variable initializations. I like that Java attempted to address the problem by providing a labeled break statement. It provides the advantage that it's structured, so you can't just arbitrarily jump anywhere, but the label is attached to the beginning of the loop, which doesn't solve the problem of having to scan up from the break statement to find the loop with that name, and then back down to find where it ends.
|
# ? Feb 27, 2010 23:14 |
|
Wheany posted:which is worse: explicit linear or explicit branched? code:
Kelson fucked around with this message at 23:37 on Feb 27, 2010 |
# ? Feb 27, 2010 23:32 |
|
Flobbster posted:I can see it being an issue if you're using preprocessor macros to do code generation, but the argument could be made that a goto with a well-named label provides better intent documentation than a mere "break;" statement with no other information. Sure. On the other hand, if you would naturally give the same well-named label to two different loops in a function, it becomes really easy to make tragic mistakes. Flobbster posted:I'd say searching for a named label is easier than starting inside a loop where the break statement is, scanning up to find the nearest while/for loop, and then scanning back down to find the matching closing brace to see where you're going to end up. I can eyeball a named label far quicker than I can pinpoint which of any number of right curly braces correspond to the exit point of a block of code. That's a fair point; break and continue are readable only if they're nested in pretty simple control flow. If you're literally just jumping out of the entire thing, goto works very well, especially if you give the label an obvious name to tells people where to start looking. On the other hand, functions do this better if you don't need to thread a huge amount of state through it (and if you aren't addicted to single-point-of-exit). Flobbster posted:I like that Java attempted to address the problem by providing a labeled break statement. This is what I meant about JavaScript — JavaScript actually allows you to attach a label to an arbitrary statement, so you can break out of an arbitrary block statement if you want without introducing a fake loop. Unfortunately, IIRC some old versions of IE didn't actually implement the spec on this and just handled the familiar Java-like cases.
|
# ? Feb 27, 2010 23:38 |
|
rjmccall posted:Sure. On the other hand, if you would naturally give the same well-named label to two different loops in a function, it becomes really easy to make tragic mistakes. code:
|
# ? Feb 28, 2010 03:15 |
|
I don't really understand all the sperging going on about this do while(false) crap. Once you see this first time, understand what it does, then see it again, it is pretty easy to follow and to understand. Speed wise it probably doesn't make any difference, so we should probably agree that this is not a coding horror, just a pattern that some people have preference for, others might not.
|
# ? Feb 28, 2010 03:22 |
|
Wheany posted:which is worse: [code snippets] I agree that both are terrible. Would it be possible to lump all of the "function"s together into a single function(), give it a pointer to the phase variable, and have it return either OK or an error code? That way, it's a single test and a single switch afterwards: code:
It's even simpler if you want to just show the code: code:
Mista _T fucked around with this message at 03:35 on Feb 28, 2010 |
# ? Feb 28, 2010 03:33 |
|
RussianManiac posted:I don't really understand all the sperging going on about this do while(false) crap. Once you see this first time, understand what it does, then see it again, it is pretty easy to follow and to understand. Speed wise it probably doesn't make any difference, so we should probably agree that this is not a coding horror, just a pattern that some people have preference for, others might not. Meh, do {...} while (0) is more commonly used as a safety hedge in function-like macro definitions
|
# ? Feb 28, 2010 04:00 |
|
Otto Skorzeny posted:Meh, do {...} while (0) is more commonly used as a safety hedge in function-like macro definitions What do you mean? Can you extrapolate?
|
# ? Feb 28, 2010 04:04 |
|
Wheany posted:which is worse: [code snippets] Mista _T posted:Would it be possible to lump all of the "function"s together into a single function(), give it a pointer to the phase variable, and have it return either OK or an error code? That way, it's a single test and a single switch afterwards: [switch snippet]
|
# ? Feb 28, 2010 04:09 |
|
Kelson posted:I can't tell if folks are being ironic in this thread any more... To back myself up, if the aim is just code efficiency, anything that reduces function calls, branching, or anything else potentially expensive in the resultant machine code is going to be better than anything anyone can cook up. If the aim is readability, then why not black-box the whole test and only offer up the result? I'm probably digging further than I need to in what probably amounts as something trivial, anyway, so I'll just back down from this one.
|
# ? Feb 28, 2010 04:31 |
|
Mista _T posted:If the aim is readability, then why not black-box the whole test and only offer up the result? I'm probably digging further than I need to in what probably amounts as something trivial, anyway, so I'll just back down from this one. because at some point you still have to read the rest of the code? What does this "black box" method end up looking like? Saying "well, just put all the code in some other method" doesn't solve anything at all. Okay, so how should I design that one?
|
# ? Feb 28, 2010 04:33 |
|
RussianManiac posted:What do you mean? Can you extrapolate? while (0) is used to allow a semicolon after the macro call, it's pretty much the only place it should be used.
|
# ? Feb 28, 2010 04:39 |
|
Ryouga Inverse posted:because at some point you still have to read the rest of the code? What does this "black box" method end up looking like? Saying "well, just put all the code in some other method" doesn't solve anything at all. Okay, so how should I design that one? "Black Box" is probably not the accepted terminology, but you can still on a per-function basis abstract out what the programmer doesn't need to know. If all that a programmer cares about is sitting in the error reporting section of the code, then that's all the programmer will want to read and not go wading through lines of "if-else" blocks. The converse, however, is that if the programmer needs to go through the code to find which part is returning an error, then they'll be reading through function call after function call attempting to follow the code, which has now been abstracted to the point of functional spaghetti. The debate in this case will always be about finding an equal balance between what the programmer needs to read and doesn't, but since you can never figure that out until some one NEEDS to read the code, what's the harm in abstracting out potentially what the programmer may not care about in function calls with descriptive names? For this case, I'm just speaking from a productivity standpoint. It's also kind of difficult to ring in on a code engineering question that only asks what type of layout is worse without stating an intended goal. If the task at hand is engineering something that a lot of people are going to see, then code design should be at the forefront of the discussion. If it is just code for a unit test, who cares what it looks like? Though if the question is just about a matter of taste, personally, I like less nested "ifs", myself.
|
# ? Feb 28, 2010 05:01 |
|
Mista _T posted:If it is just code for a unit test, who cares what it looks like? People who want to check if the unit test is correct? That's a hugely important factor in in lots and lots of projects. If you ever write code thinking 'no-one will need to read this, it's fine', you're almost certainly wrong.
|
# ? Feb 28, 2010 05:21 |
|
Mista _T posted:To back myself up, if the aim is just code efficiency, anything that reduces function calls, branching, or anything else potentially expensive in the resultant machine code is going to be better than anything anyone can cook up. If the aim is readability, then why not black-box the whole test and only offer up the result? I'm probably digging further than I need to in what probably amounts as something trivial, anyway, so I'll just back down from this one.
|
# ? Feb 28, 2010 05:35 |
|
Jonnty posted:People who want to check if the unit test is correct? That's a hugely important factor in in lots and lots of projects. If you ever write code thinking 'no-one will need to read this, it's fine', you're almost certainly wrong. What I mean by "who cares what it looks like" is in the context of the question asked and not of the user feedback. OBVIOUSLY, the point of a unit test is to provide some kind of feedback to - at the very least - the programmer writing the test. The question asked seemed to be more about code structure rather than feedback, which I admit I commented on incorrectly in the first of my series of posts. Sorry if it was ambiguous. From reading Kelson's feedback a couple posts up, it is clear that you can't just pose a question about code layout and structure without providing context, so there isn't any need to get testy (pun almost certainly NOT intended) about what I said in my previous post which apparently it seems you interpreted is an affront to unit testing procedure. Kelson posted:To reiterate Ryouga's point, your suggestion just offloads the code/work to an as-yet-undesigned function. Given the only purpose of this function was, apparently, to execute the required functions in a given order given successful executions or error and print a descriptive report... what would the other function do? Execute the required functions in a given order given successful executions or error and return a sufficiently descriptive error code, which the other function turns into a descriptive report? The other function would do the same, except it would only return once and the condition of "status" would only be checked once. It's a pet peeve of mine to see a lot of "ifs" in a row that still test a single value when all we need is what went wrong in a series of tests ("switch" is implemented different ways by different compilers, so even my solution may not hold up for this reason). If the test fails after function1() in Wheazy's first example, you are still testing "status" an unnecessary number of times. In the second example, you don't test it more times than you need to, but you still have an ugly mess of nested "ifs". I dunno. It REALLY pays to have an actual example of code in this case instead of assuming what each "function" returns. In theory, it's always better to go with what is readable, but without seeing the big picture, all that "Which is worse?" is going to get are long discussions with a load of assumptions. I admit that I got ahead of myself with immediately reorganizing the structure of the procedure itself in order to facilitate readability of the outcome. If the goal was just code structure around a series of calls and halting the test after an error, then both examples still work, more or less, but then it just becomes a question of taste, which I can ONLY assume at this point. Mista _T fucked around with this message at 05:58 on Feb 28, 2010 |
# ? Feb 28, 2010 05:37 |
|
Assuming OK==0code:
|
# ? Feb 28, 2010 06:30 |
|
Kelson posted:Assuming OK==0 I do believe what I am looking at is coding perfection, my good man.
|
# ? Feb 28, 2010 06:48 |
|
Ryouga Inverse posted:
Labels are local to functions so this is wholly unnecessary.
|
# ? Feb 28, 2010 06:53 |
|
RussianManiac posted:What do you mean? Can you extrapolate? Are you seriously incapable of Googling "do while macro"?
|
# ? Feb 28, 2010 06:53 |
|
MrMoo posted:while (0) is used to allow a semicolon after the macro call, it's pretty much the only place it should be used. Another piece of ammunition for my next anti macro rant.
|
# ? Feb 28, 2010 07:04 |
|
Factor Mystic posted:Another piece of ammunition for my next anti macro rant. Congrats on being ignorant.
|
# ? Feb 28, 2010 07:05 |
|
|
# ? May 15, 2024 03:23 |
|
Mista _T posted:If the test fails after function1() in Wheazy's first example, you are still testing "status" an unnecessary number of times. That said, my third idea was having an array of function pointers, with the last array entry being NULL, then doing a code:
|
# ? Feb 28, 2010 09:35 |