Re: Code Comprehension
- From: Logan Shaw <lshaw-usenet@xxxxxxxxxxxxx>
- Date: Fri, 25 Aug 2006 05:12:07 GMT
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
.
- Follow-Ups:
- Re: Code Comprehension
- From: Simon Morgan
- Re: Code Comprehension
- From: Pascal Bourguignon
- Re: Code Comprehension
- References:
- Code Comprehension
- From: jj
- Re: Code Comprehension
- From: Pascal Bourguignon
- Code Comprehension
- Prev by Date: Re: Code Comprehension
- Next by Date: Re: Code Comprehension
- Previous by thread: Re: Code Comprehension
- Next by thread: Re: Code Comprehension
- Index(es):
Relevant Pages
|