• Michael R (unregistered)

    ++zer0th

  • LZ79LRU (unregistered)

    Honestly I don't hate this. It looks like code written by a relative beginner who couldn't spot his own error and yet managed to figure out a fix eventually. Is it good code? Hell no. But it is code by someone who at the very least tried hard. And I see potential in that. Like if I was grading a class in programming 101 I would pass his.

    And it's not like this is something seriously written by a professional for production. Right? Right?

  • Labasta (unregistered) in reply to LZ79LRU

    Sorry no, I don't care how much of a beginner the perpetrator of this is. I would definitely dock marks if it came to me for evaluation.

    I might even use it as an exercise in a programming 101 class as a "What's wrong with this picture?" example.

  • (nodebb) 
        for(int i=0; i<values.size(); i= i++)
    {
        map.put(values.get(i), values.get(++i));
        i++;
    }

    I hope this isn't C++ (earlier versions for sure, but C++17 changed the rules in ways that might exclude certain parts of this analysis), because if it is, it is a Glastonbury-scale festival of undefined behaviour.

    i = i++; modifies i twice, and might, if the compiler is feeling malicious, be equivalent to i++.

    The order in which the arguments to map.put() are evaluated is not specified by the standards, so it's not clear whether (if i is initially 1) we pass item 1 and item 2, or item 2 and item 2. (Relying on the order causes UB.)

  • Robin (unregistered)

    "It's like ChatGPT, but less self-aware."

    ChatGPT has zero awareness, so while this developer has seemingly close to that it's hard to imagine it's actually less.

  • ISSOtm (unregistered) in reply to Steve_The_Cynic

    Actually, i= i++; is guaranteed to be a no-op. According to N1570 (the C11 final draft), 6.5.16 § 3:

    [...] The side effect of updating the stored value of the left operand is sequenced after the value computations of the left and right operands. [...]

    So i++ evaluates to i pre-increment, and has the side effect of incrementing i; but then i is assigned back the original value (and that's well-defined because of the above clause).

    The map.put() line is however a classical example of unsequenced evaluation, but that is "only" unspecified, not undefined (according to my reading of 5.1.2.3 § 3, anyway... god this standard is convoluted. :/). Not that this makes much difference.

    Still, this is something I'd only see having a place in the Underhanded C Contest, not quiiiite in production :)

  • ISSOtm (unregistered) in reply to LZ79LRU

    I personally do hate this, because it manages to trigger undefined (or at least unspecified, c.f. my other comment) behaviour in one place, and only narrowly dodge it in a second one. C requires a lot of discipline to avoid triggering UB (which a different compiler, or a later version of the same, will eventually subtly break, much to everyone's confusion and dismay), so I wouldn't pass this code even in a CS 101 course.

    Of course, TRWTF may be the C standard committee's reluctance to adapt the language's semantics (hey, it only took them a decade or two to stop supporting one's complement and sign-magnitude! And only one more to get rid of pre-standard function declarations!), and/or using C to teach CS 101.

    ...would now be a good time to shill Rust? 😁

  • COB666 (unregistered)

    That code looks like it shouldn't work as described. If I was doing a code review on that, I'd either re-write it so it actually looked like how it was supposed to function or ask the developer to walk me through their logic for writing it.

  • Stella (unregistered)

    There is a lot more WTF to this. It's a wonder this works at all.

    map.put(values.get(i), values.get(++i)); is undefined behaviour, because the compiler is free to order the evaluation of parameters. It may increment i before it gets passed to values.get(i), or it may not. Thus, it's basically bound to a specific compiler. Have fun tracking down the bugs when changing compiler versions.

  • Anonymous') OR 1=1; DROP TABLE wtf; -- (unregistered) in reply to ISSOtm

    This is highly unlikely to be C code. It technically could be, if map were a structure containing a function pointer named put, but that would be highly unusual given that it doesn't pass map as a parameter, so it'd have to be a global mapping object.

    It could be C++, but more likely it's Java, using the standard Map interface. And in Java, the expression i = i++ has well-defined behavior: it increments i and returns the old value of i in evaluating i++, and then it assigns the old value of i to itself, thereby having no change in the value of i.

  • Conradus (unregistered) in reply to ISSOtm

    "UB (which a different compiler, or a later version of the same, will eventually subtly break, much to everyone's confusion and dismay),"

    No, that's "implementation dependent". "Undefined behavior" means it can change because of what else is running on the machine, subtle timing issues, the way you held your mouth, or for no reason at all.

  • LZ79LRU (unregistered) in reply to Labasta

    I said pass, not mark highly. Like this would be a minimum points pass with a note to see me after class. But it wouldn't be a fail.

  • LZ79LRU (unregistered)

    Also, I assumed this was neither C nor C++ but maybe C# or similar due to the fact that the article made no mention of it being dumb on account of UB.

    This said, I absolutely do think that a persons first language should be C. The proper way to learn programming is to understand the base concepts of what you are doing. And you can only get there with low level languages.

    Starting with a high level language teaches you how to put together blocks of someone elses code that you neither understand nor care to and produce results that are cargo cult implementations of "don't question it as long as it works".

    And C is the ideal candidate because it is as low level as it gets while still being abstract enough that it is not hardware dependent.

  • Tim R (unregistered)

    ugh - I hate for loops in almost all cases. surely even in C there's some kind of functional programming library available that would give you 'partition' and 'zip'

    in JavaScript this would be something like (untested):

    const pairs = .unzip(.partition(values, x=> !(x%2))

  • David Brown (unregistered) in reply to ISSOtm

    That same section also states that "the evaluations of the operands are unsequenced". In addition in 6.5 paragraph 2:

    "If a side effect on a scalar object is unsequenced relative to either a different side effect on the same scalar object or a value computation using the value of the same scalar object, the behavior is undefined."

    Which suggests to me that this is still undefined.

  • Randal L. Schwartz (github)

    I probably would have written (in Dart, because I don't quite know the OP language):

    for (final i = 0; i < input.size; ) { final first = input[i++]; final second = input[i++]; doSomethingWithThePair(first, second); }

    In this case, input[i] is invariantly "the first unprocessed item of input". And the third part of the 3-part for-loop has nothing to do. In fact, except for the local i, this could have been a while loop instead.

    Addendum 2023-07-05 12:04: I've found that thinking about invariants in loops is not just an academic exercise, it's actually quite practical.

  • Jonathan (unregistered)

    "++" always makes you have to think extra. Why not

    for(int i=0; i<values.size(); i += 2)

    {

    map.put(values.get(i), values.get(i+1));

    }

    The "+= 2" makes it clear that you are processing them two at a time.

    (This is the first time I've put code in a comment here. Expect tears.)

  • Remcoder (unregistered)

    About the easy reader comment.

    I would think ChatGPT is already trained with the code from this site. It was trained with basically the whole internet as a dataset, which would include this site.

    So, there already is an LLM in existence which was trained using code from this site.

  • (nodebb) in reply to ISSOtm

    So i++ evaluates to i pre-increment, and has the side effect of incrementing i

    The side effect you mentioned isn't part of the "value computations".

    cppreference.com observes this about the C rule you cited:

    The value computations (but not the side-effects) of the operands to any operator are sequenced before the value computation of the result of the operator (but not its side-effects).

    https://en.cppreference.com/w/c/language/eval_order

    Note: not the side-effects. They are sequenced whenever the compiler feels like making them happen, which is why i = i++ is UB.

    But of course I was talking about C++, so a discussion of the fine-grained details of C is of limited relevance.

  • Maia Everett (github) in reply to Anonymous') OR 1=1; DROP TABLE wtf; --

    It's likely Java. Java has a Map type with a put method. It's unlikely to be C#, because in .NET, maps are called dictionaries, and method names begin with an uppercase letter.

  • BacklinksProvider123 (unregistered)
    Comment held for moderation.
  • (nodebb)

    If you really want to separate the wheat from the chaff, ask them to write a for loop that counts by two, or three, or some other arbitrary number.

    10 FOR N = 1 TO 10 STEP 2; PRINT N; NEXT N

    Addendum 2023-07-05 13:28: Of course, the ; should be : else it’s not going to work … Damn :(

  • mihi (unregistered) in reply to LZ79LRU
    Comment held for moderation.
  • Anon55 (unregistered) in reply to ISSOtm
    Comment held for moderation.
  • NoLand (unregistered) in reply to Gurth
    Comment held for moderation.
  • LZ79LRU (unregistered) in reply to Gurth

    myLoop mov cx, 0 // do stuff add cx, [increment] cmp cx, [limit] jle myLoop

    xD

  • ISSOtm (unregistered) in reply to Conradus

    UB is a subset of "implementation-defined", as what a compiler does to a certain UB code path may vary across its versions. (Note further that implementations are allowed to be stricter than the standard, i.e. to define what would otherwise be undefined! GCC does this, for example, by treating <negative value> >> n as a sign-extending right-shift. C.f. their implementation-defined behaviour docs)

    The most famous example of this, I think, has to be newer versions of GCC making some "eager" pointer derefs optimising out NULL checks in the Linux kernel, which caused quite the turmoil!

  • ISSOtm (unregistered) in reply to Steve_The_Cynic
    Comment held for moderation.
  • DeeKay (unregistered) in reply to Gurth

    Moving from BASIC to Pascal I was shocked that there was no "STEP" option in Pascal and that a for loop in Pascal only worked on ordinals. Seemed so archaic. Until I realised what other options opened up due to dropping BASIC, and yes, maybe I shouldn't use the for loop all that often.

  • Just another Embedded Designer (unregistered)

    Why am I having Deja Vu ??

    I m sorting out code where one "developer" or "code monkey" would be rating them too highly, claims in code things like

    i = i + 1;   // i++ will not work here....
  • (nodebb)

    Okay, always, always, always cache result values before using them in the condition of an iteration. By definition all C-style languages reevaluate the condition in an for-statement each single time, so never do something like

    for(index = 0; index < collection.get_length(); index++) {  ... }

    but instead use

    int length = collection.get_length();
for(index = 0; index < length; index++) {  ... }

    The reason is pretty simple: Because by definition the value is allowed to change the compiler can't optimize this step and it will always make the call in every iteration. Not only is the call itself a waste of resources, if there is something more complex going on in this method it can be even worse up to potential deadlocks in some scenarios. So it's really, really bad practice to do so.

  • Anonymous') OR 1=1; DROP TABLE wtf; -- (unregistered) in reply to MaxiTB
    Comment held for moderation.
  • Jeremy (unregistered)

    As mentioned numerous times the usage of i++ has some issues. But whytf is no one looking at the other part of the for loop? 'for(int i=0; i<values.size();' Inside the loop you will be accessing values out of bounds of your stop condition. Has no one here learned not to trust input or in this case input size?

    Perhaps for(int i=0; ++i<values.size();i++) { would be more intuitive to use with acces to values(i-1) and values(i) resolving any ambiguous execution order of map

Leave a comment on “Twice, Thrice”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article