* Kragen Javier Sitaker <[EMAIL PROTECTED]> [2008-07-03 09:40]:
> I agree that the code needs rewriting, but inspired by Scheme,
> I would do it without magic numbers, without nine separate
> variables for the struct options pointers, and with conditions
> and their consequents on the same line:

Your version isn’t tabular though. This is something I like about
both of the other rewrites better than yours. However, Spinellis
waffles on way too long to determine which array element to pick
and Steele commits the terrible sin of copypasting the x-related
conditional onto every one of the three lines.

Here’s how I’d fix both of these problems:

    struct options *locations[3][3] = {
         {upleft,  upper,  upright },
         {left,    normal, right   },
         {lowleft, lower,  lowright},
    };
    int h = x == last   ? 2 : !!x;
    int v = y == bottom ? 2 : !!y;
    op = &(locations[v][h])[w->orientation];

This makes use of magic numbers and exploits the C type system
quirks relating to ints and boolean logic. Is that bad style?

Or is it idiomatic C?

In any case it’s short enough to grasp quickly and the central
intent of the code is communicated visually by a table. I believe
quite strongly that code driven by tabular declarative sections,
even if the imperative part is somewhat tricky or even messy, is
easier to understand than simpler but more abstractly written code.

I do not see this as the be-all end-all version either, btw; if
there was a way to get the offset of a named member of a struct
in C, it would be possible to meld the advantages of your code
and mine. Think of the machine code that would be generated from
my code: there will multiplications with sizeof(options) in
there. The machine code version of your code would do the job
directly.

-- 
*AUTOLOAD=*_;sub _{s/(.*)::(.*)/print$2,(",$\/"," ")[defined wantarray]/e;$1}
&Just->another->Perl->hack;
#Aristotle Pagaltzis // <http://plasmasturm.org/>

Reply via email to