Re: Code Comprehension



Pascal Bourguignon wrote:
So that gives:

void fun(char *a, const char *b) {
int apos = 0, i, j;
if((a==NULL)||(b==NULL)){return;} for(i=0;a[i]!='\0';i++){
for(j=0;(b[j]!='\0')&&(a[i]!=b[j]);j++);
if(b[j]!='\0'){
a[apos++] = a[i]; }}
a[apos]='\0'; }

My god! This is twice as hard to read! You do know that whitespace
is allowed in C, right? :-) No wait, forget the smiley. I'm
serious. That is visual clutter at its worst. Whitespace is
legal for a reason, and that is to help make things clearer
by allowing the formatting to be used as a tool to help show
the structure.

If you don't like the idiomatic parts of C and you just want to
clean it up a little, may I suggest:

void fun (char *a, const char *b)
{
int apos = 0;
int i, j;

if (a == NULL || b == NULL) { return; }

for (i=0; a[i] != 0; i++)
{
for (j = 0; b[j] != 0 && a[i] != b[j]; j++)
;

if (b[j] != 0) { a[apos++] = a[i]; }
}

a[apos]='\0';
}

Yes, that still retains some C idioms, but you have to assume
some familiarity with the language. (I noticed even you did
that, because you did not eliminate the "for" loop, which is
deep in the C idiom, and replace it with a "while" loop.)
Things like the low precedence of "||" and the fact that 0 by
itself is a legal character constant aren't very complicated
and are hard to miss if you have much experience with C.

And anyway, I really doubt the more obvious C idioms are a
significant reason why many would find this code hard to read.

I've also put the ";" on its own line to draw as much attention
as possible to the fact that the "for" loop has a null body.
Putting the ";" right up against the ")", to me, just hides
that fact, which is something that needs to be highlighted.

Here is a version that might be a little easier to understand:

void fun (char *target, const char *setofchars)
{
const char *source;

if (a == NULL || b == NULL) { return; }

for (source = target; *source != 0; source++)
{
if (strchr (setofchars, *source) != NULL)
{
*target = *source;
target++;
}
}

*target = 0;
}

If one likes to play the minimalism game, that can be done
and still keep it readable:

void fun (char *target, const char *setofchars)
{
const char *source;

if (a == NULL || b == NULL)
return;

for (source = target; *source != 0; source++)
if (strchr (setofchars, *source) != NULL)
*(target++) = *source;

*target = 0;
}

- Logan
.



Relevant Pages

  • Re: Code Comprehension
    ... Reproducing the example for comparison ... void fun(char *a, const char *b) { ... int apos = 0, i, j; ...
    (comp.programming)
  • Re: Code Comprehension
    ... int apos = 0, i, j; ... void fun(char *a, const char *b) { ... The forloop finds the position of the first character in b ... void remove_characters{ ...
    (comp.programming)
  • Re: texting c and c++
    ... aka MLO or I3NOO ... template void valnam( ... genex(const char * w, const char * f, unsigned int l) ...
    (comp.unix.programmer)
  • [PATCH 4/4] m68k: cleanup inline mem functions
    ... -int memcmp(const void * cs,const void * ct,size_t count) ... const unsigned char *su1, *su2; ... size_t temp, temp1; ... const char *cfrom = from; ...
    (Linux-Kernel)
  • [PATCH] adjust _acpi_{module,function}_name definitions
    ... u32 line_number, ... va_list args; ... const char *function_name, ...
    (Linux-Kernel)