Path: csiph.com!eternal-september.org!reader02.eternal-september.org!.POSTED!not-for-mail
From: Tim Rentsch <tr.17687@z991.linuxsc.com>
Newsgroups: comp.lang.c
Subject: Re: A question about style
Date: Thu, 30 Dec 2021 14:01:06 -0800
Organization: A noiseless patient Spider
Lines: 84
Message-ID: <86ee5tzra5.fsf@linuxsc.com>
References: <sqjivq$get$1@oguzismail.eternal-september.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Injection-Info: reader02.eternal-september.org; posting-host="0a3a87556a94a586a5bc6690bb7b95d8"; logging-data="420"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX1+YMSvVFwQFb9XoclqyVQNs56oWnU9ei1A="
User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.4 (gnu/linux)
Cancel-Lock: sha1:3F/ifDN/qu0XBN1ASUkbhJxRAHo= sha1:CpJniICbvvVze1F9BhC+4mG7Hd4=
Xref: csiph.com comp.lang.c:164134

Oğuz <oguzismailuysal@gmail.com> writes:

> I have this if-else thingy:
>
> if (!expr[expr_length - 1].is_number) {
>   if (skip_adj[expr[expr_length - 1].value][op])
>     return false;
> }
> else if (expr_length > 2 && !expr[expr_length - 2].is_number) {
>   if (skip_alt[expr[expr_length - 2].value][op])
>     return false;
>   else if (expr[expr_length - 3].is_number
>       && expr[expr_length - 3].value < expr[expr_length - 1].value
>       && skip_alt_asc[expr[expr_length - 2].value][op])
>     return false;
> }
>
> (`expr' is a fixed-size array of structs, I increase `expr_length'
> as I fill it in.  `skip_*' are two dimensional arrays of booleans,
> but I don't have any problem regarding them.)
>
> And it looks ugly;  barely readable even with comments.  I want it
> to be readable at least.  [...]

Putting the code in the context of a function body (with a
'return 5;' at the end just to pick a value obviously not
the same as 'false'):

    int
    original_code(){
      if (!expr[expr_length - 1].is_number) {
        if (skip_adj[expr[expr_length - 1].value][op])
          return false;
      }
      else if (expr_length > 2 && !expr[expr_length - 2].is_number) {
        if (skip_alt[expr[expr_length - 2].value][op])
          return false;
        else if (expr[expr_length - 3].is_number
            && expr[expr_length - 3].value < expr[expr_length - 1].value
            && skip_alt_asc[expr[expr_length - 2].value][op])
          return false;
      }
      return 5;
    }

Assuming no major changes to the code (so stipulating any changes
be kept local to this one function), and taking 'expr' to be an
array of type 'Expression', I would try something along these
lines:

    int
    possible_revision(){
        Expression *e = expr + expr_length;

        if(  e[-1].is_number  ){
            if(  skip_adj[ e[-1].value ][op]  )  return  false;

        } else if(  expr_length > 2  &&  e[-2].is_number  ){
            if(
                skip_alt[ e[-2].value ][op]  ||  (
                    e[-3].is_number               &&
                    e[-3].value  <  e[-1].value   &&
                    skip_alt_asc[ e[-2].value ][op]
                )
            ){
                return  false;
            }
        }

        return  5;
    }

To me this way of writing is cleaner and easier to take in.  I
have the sense that I can follow what is being done, even if
exactly what is being accomplished for the caller isn't evident.
Choosing a good name for the function might to a long way towards
addressing the latter question.

Different people have different reactions to various lexical
styles, so that aspect may be somewhat off putting here.  If so
then perhaps the example will spark some other ideas.  My sense
for this question is that a judicious choice of where to insert
white space, both horizontal and vertical, is a key element of
making the code easier to take in and comprehend.