• (nodebb)

    I just hope that when the code says 65, 66, 67, 68, etc., it doesn't really mean 'A', 'B', 'C', 'D', ...

  • DQ (unregistered) in reply to Steve_The_Cynic

    Idle hope, I think.

  • Industrial Automation Engineer (unregistered)

    Stretching the mind: the dimension (i.e. unit) of 'val' can be either [errorcode] or [keycode] in that sense I might be able to understand how the program hierarchy attempts to make that distinction. That is, when ignoring the fact of storing values of a different type into a variable, or the poor hierarchy used.

    ( Yes, this comment is TRWTF)

  • WTFGuy (unregistered)

    I know nothing of the Curses library beyond a couple minutes on wiki & Googling up some docs to read. Which were ... quaint.

    I almost wonder if the outer switch was an afterthought. IOW, they had their "nice" organized switch on keyvalue and then something changed and now ERR was occasionally leaking through to this part of the program. So the person tasked with fixing this bug didn't bother to understand all the ramifications of the existing code, they just wrapped the whoe swith in what amounts to if (val != ERR) {... whole 3 pages of switch here ...}

    One of the reasons I suggest this is the Curses function that returns keypresses can be called in either blocking or non-blocking mode. In blocking mode it waits indefinitely for a keypress and cannot return ERR. In non-blocking mode it returns immediately and if the keyboard buffer was empty it returns ERR.

    I'm imagining somebody with a single-threaded single-windowed (single-screened really) mentality having written a whole app in a blocking vernacular. The somebody realized it needs to be non-blocking, switched the global mode (of course it's a global mode) of CurGetch() calls to non-blocking during the app init, and unleashed a bugfest throughout the app. Which were duly band-aided like this.

  • RLB (unregistered)

    I bet I know what happened. It started out as if (val == ERR) then { error code} else switch (val) { case scancode: ... }. The next programmer saw this and noticed that that might as well be one switch. So he corrected it... almost, but not quite.

  • van Dartel (unregistered)

    It's fine (even great) to have syntactically distinguished paths for handling exceptional and normal flow. However, the code seems to assume that all erroneous inputs have already been detected and (apparently) such inputs are nevertheless allowed to propagate through the system (as ERR). For the code in question, that makes ERR an ordinary citizen of the input space and therefore ERR should've been treated indiscriminately from other values.

    That was perhaps a bit too much in terms of connotations that might follow ;-)

    Here's something less loaded: why are there magic values like 65? (and not implicit values like curses.KEY_DOWN, which BTW is not even close to 65)

  • (nodebb)

    A C Switch (in modern compilers) can be implemented as either a series of compares (if/else, effectively) or a table lookup (there are a few other ways, but these are relevant). The choice is often made on the density of the values within the switch. The 4 cases for the arrows, appear to be a good choice for a lookup, but ERR is likely to be outside that range.

    This implementation may become a single if followed by an indexed JMP. IF the "trivial" approach was used, the entire tree could degrade to if/elseif/elseif/elseif....

  • Foo AKA Fooo (unregistered)

    We don't know what's in the skipped parts. Maybe it was:

    case ERR:
            while (val == ERR) val = getch ();

    with fallthrough, so the inner switch gets a different value of val. In this case, the code would be ... well, still stupid, but not equivalent to a single switch.

    Yeah, I guess, TheCPUWizard's explanation is more likely, attempted (and undocumented) micro-optimization for last millennium's compilers.

  • van Dartel (unregistered) in reply to TheCPUWizard

    If k of N branches are exceptional (the other being consecutive), an optimal re-branching that collapses N-k cases into a trivial lookup/JMP by offset can easily be determined in O(N^(k+1)) using the most basic implementation I can think of. In this case, k<=1 so it would have been cheap to optimize. But you may be right; perhaps compilers are not overly interested in this kind of local optimizations.

  • ooOOooGa (unregistered) in reply to TheCPUWizard

    This implementation may become a single if followed by an indexed JMP.

    One of the few times I would want to see compiler optimizations explicitly written into the code. I wouldn't want to write an unrolled loop explicitly. But if this code was actually an 'if' for the ERR case and then the nice, dense switch that gets compiled to a vector lookup - that would be good code.

  • WTFGuy (unregistered)

    Trying to out-optimize the compiler using obscure knowledge of how the source language might possibly be compiled when we're dealing with UI code is a WTF. IMO aAnyone thinking along those lines was a WTF at the center of their own thought processes.

    The measures of merit of UI code are Correctness, Clarity, and Consistency. Any other considerations are a distraction from the Big 3.

  • (nodebb) in reply to TheCPUWizard

    I wonder if modern compilers will recognize a switch that's "mostly dense" and compile to a mixture of tests and jump tables.

  • Gary (unregistered)

    It's also possible that there was some common code after the inner switch, so that a break in one of the character cases would run that common code section, but a break in the error case would not. Even that would be, shall we say, not the best way to design the code.

  • Loren Pechtel (unregistered)

    Another possibility comes to mind:

    The outer switch only has ERR. What if it had other stuff in the past? The nested switches could be so the compiler would optimize (don't look at modern compilers, what matters is what the compiler did when this was written.)

    I can see this being the result of succession of changes without anyone being crazy.

  • (nodebb) in reply to Barry Margolin 0

    I wonder if modern compilers will recognize a switch that's "mostly dense" and compile to a mixture of tests and jump tables.

    Probably. And in the context, "modern" probably means "in the last quarter of a century or so".

  • RLB (unregistered) in reply to WTFGuy

    Hm. In practice, Efficiency is also rather important. Not always - premature optimisation an'allthat - but yeah, if your program is so slow that your users get irritated...

  • (nodebb) in reply to TheCPUWizard

    A C Switch (in modern compilers) can be implemented as either a series of compares (if/else, effectively) or a table lookup (there are a few other ways, but these are relevant).

    Modern compilers will also swap between the two for code issuing as they see fit. And yes, that means they can compact if-elseif-else chains into a jump table in some cases.

  • Sigako (unregistered) in reply to Steve_The_Cynic

    I know that these values correspond to F7, F8, F9, F10 keycodes on standard Windows keyboard. Don't know whether it's the case.

  • ooOOooGa (unregistered) in reply to WTFGuy

    That is actually what I was saying. Having the ERR case as its own if statement before the switch would still be good code even without taking compiler optimizations into account. Having the ERR case as just another case in the switch statement would also work. Not sure how that would affect the compiler, but that is much less important to me.

  • Paul (unregistered) in reply to Steve_The_Cynic

    Seeing as curses was first written in the late 1970s, that might have been a reasonable optimisation back then.

  • Royal Stitches (unregistered)
    Comment held for moderation.
  • downloadcracker (unregistered)
    Comment held for moderation.
  • downloadcracker (unregistered)
    Comment held for moderation.
  • Some Ed (unregistered) in reply to TheCPUWizard

    Additionally, a good compiler can merge the two switch statements into one if that results in better assembly.

    I recall back in the day being told that it was called 'curses' because it focused on dealing with a problem that caused people to curse a lot, and while the goal was to confine the cursing into this one library, it quickly became apparent that anyone who used curses would sooner or later do some cursing themselves.

    Code like this was pretty common for stuff involving curses, and sometimes caused a certain amount of it. Yes, it's a WTF, but at one point, it was at least a defacto industry standard WTF.

Leave a comment on “A Real Switcheroo”

Log In or post as a guest

Replying to comment #:

« Return to Article