code review

[E. Robert Tisdale]

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);
if(t != NULL)
{
char *u;
strcpy(t, s);
u = t;
while(*u)
{
if(*u == '.')
{
*u = '_';
}
++u;
}
}
return t;
}


Is there any way to improve the code above?

 

[Peter Pichler]

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);
if(t != NULL)
{
char *u;
strcpy(t, s);
u = t;
while(*u)
{
if(*u == '.')
{
*u = '_';
}
++u;
}
}
return t;
}

Is there any way to improve the code above?

You need to ask? (For the benefit of newcomers, ERT claims to be an expert
on C.)

It can be improved by not going through the string twice.

 

[Leor Zolman]

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);
if(t != NULL)
{
char *u;
strcpy(t, s);
u = t;
while(*u)
{
if(*u == '.')
{
*u = '_';
}
++u;
}
}
return t;
}

Probably not _much_ of an improvement, but I noticed you make two
passes over the source text, one within strcpy and then again in your
own loop. Here's a one-pass solution:

char *dot_to_underscore2(const char *s)
{
char *result;

if ((result = malloc(strlen(s) + 1)) != NULL)
{
char const *src = s;
char *dest = result;
char c;

for (; (c = *src) != '\0'; src++, dest++)
*dest = (c == '.') ? '_' : c;
*dest = '\0';
}

return result;
}


Leor Zolman
BD Software
(e-mail address removed)
www.bdsoft.com -- On-Site Training in C/C++, Java, Perl & Unix
C++ users: Download BD Software's free STL Error Message
Decryptor at www.bdsoft.com/tools/stlfilt.html

 

[Morris Dovey]

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);
if(t != NULL)
{
char *u;
strcpy(t, s);
u = t;
while(*u)
{
if(*u == '.')
{
*u = '_';
}
++u;
}
}
return t;
}


Is there any way to improve the code above?

It would probably be much improved by including stdlib.h - I'd
probably have written:

#include <stdio.h>
char *dot_to_underscore(const char *s)
{ char c,*t=s,*u;
if (s && (t = u = malloc(strlen(s) + 1)))
{ while (c = *s++)
{ if (c == '.') c = '_';
*u++ = c;
}
}
return t;
}

but there's no way to be sure of any real improvement in either
size or performance.

 

[Morris Dovey]

An afterthought...

Probably be a good idea to include string.h, too.

 

[Eric Sosman]

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);
if(t != NULL)
{
char *u;
strcpy(t, s);
u = t;
while(*u)
{
if(*u == '.')
{
*u = '_';
}
++u;
}
}
return t;
}

Is there any way to improve the code above?

Since "goodness" is multi-dimensional, "improve" is
ambiguous. However, one possible improvement would be to
combine the translation with the copy operation:

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);
if (t != NULL) {
char *u = t;
for ( ; ; ++s) {
if ((*u++ = *s == '.' ? '_' : *s) == '\0')
break;
}
}
return t;
}

(The loop could, of course, be written more compactly
at some cost in readability.)

Because of the multi-dimensionality mentioned above, no
consideration will be given to any discussion of whether this
sort of thing represents an "improvement."

 

[Peter Pichler]

It would probably be much improved by including stdlib.h - I'd
probably have written:

So why don't you include it? ;-)

#include <stdio.h>
char *dot_to_underscore(const char *s)
{ char c,*t=s,*u;
if (s && (t = u = malloc(strlen(s) + 1)))
{ while (c = *s++)
{ if (c == '.') c = '_';
*u++ = c;
}
}
return t;
}

but there's no way to be sure of any real improvement in either
size or performance.

Have you ever heard about profilers? ;-)
(No, I don't even pretend to be serious in *this* thread.)

 

[Morris Dovey]

Have you ever heard about profilers? ;-)
(No, I don't even pretend to be serious in *this* thread.)

Peter...

Of course - but I haven't been able to find a /generic/ profiler
with any of the search engines available to me.

Now /that/ would be an interesting project...

 

[Leor Zolman]

An afterthought...

Probably be a good idea to include string.h, too.

Is there some sort of guideline in this group that makes posting a
free-standing, self-explanatory little function using totally standard
lib calls, without the requisite #includes, into a major faux pas? I
mean, I get that some folks may not like E. Robert Tisdale for one
reason or another and need to bash him, but out of that context it
just seems ugly. In it too, for that matter.
-leor


Leor Zolman
BD Software
(e-mail address removed)
www.bdsoft.com -- On-Site Training in C/C++, Java, Perl & Unix
C++ users: Download BD Software's free STL Error Message
Decryptor at www.bdsoft.com/tools/stlfilt.html

 

[Richard Heathfield]

Is there some sort of guideline in this group that makes posting a
free-standing, self-explanatory little function using totally standard
lib calls, without the requisite #includes, into a major faux pas?

No, but de facto faux pas it is, nevertheless. When I'm composing code for
posting on comp.lang.c, I generally err on the side of caution and try to
make it totally nitpick-proof. I know I don't always succeed because I have
all these scars.

I
mean, I get that some folks may not like E. Robert Tisdale for one
reason or another and need to bash him, but out of that context it
just seems ugly. In it too, for that matter.

It's nothing to do with Tisdale. It's to do with accuracy. Yes, it may seem
a bit petty, and yes, sometimes we don't bother (and of course we wouldn't
do this for an obviously exegetic fragment), but it is useful because it
makes people more conscious - more careful - about what they're writing.

Personally, I've found that this has a /positive/ effect on my programming.
When I'm writing code and find myself taking a little shortcut, I think to
myself "what would comp.lang.c say?", and Do the Right Thing instead.

So, all in all, I think the nitpicking is a good thing, not a bad thing.

 

[Morris Dovey]

Is there some sort of guideline in this group that makes posting a
free-standing, self-explanatory little function using totally standard
lib calls, without the requisite #includes, into a major faux pas? I
mean, I get that some folks may not like E. Robert Tisdale for one
reason or another and need to bash him, but out of that context it
just seems ugly. In it too, for that matter.

Leor...

Nothing to do with the OP - and I'm not much of a "basher". If
even a small and extremely simple program requires a standard
header, then the code is incomplete without it.

I made the same mistake that the OP made ( as Peter so kindly
pointed out.

I try to remember that there are lurkers in the shadows - people
who are just starting to learn C and who need to understand that
inclusion of proper header files is a necessary part of writing C
code. My personal rule of thumb is that if I post a complete
function, then the required headers should be shown. If it's just
a fragment of a function, then the headers aren't needed.

At one time or another I think every single C regular has pointed
out that I've forgotten a necessary header - you'd think I could
get it right by now...

 

[Leor Zolman]

Leor...

Nothing to do with the OP - and I'm not much of a "basher". If
even a small and extremely simple program requires a standard
header, then the code is incomplete without it.

I made the same mistake that the OP made ( as Peter so kindly
pointed out.

I try to remember that there are lurkers in the shadows - people
who are just starting to learn C and who need to understand that
inclusion of proper header files is a necessary part of writing C
code. My personal rule of thumb is that if I post a complete
function, then the required headers should be shown. If it's just
a fragment of a function, then the headers aren't needed.

At one time or another I think every single C regular has pointed
out that I've forgotten a necessary header - you'd think I could
get it right by now...

Okay, I hear you (and Richard). Every time I select Agent's Send Now
menu option (or hit control-N, which I discovered by accident just a
few days ago is equivalent), I steel myself up for the possibility of
regretting it ;-). But after several months of posting, for better or
worse, I can state that there's not one single response to any of my
posts, even the seriously brain-dead ones, that I ended up feeling
"attacked" over. I wonder whether my having at least a little bit of
name recognition (and certainly not to everybody) has tempered what
might otherwise have been more acerbic reactions, but for whatever the
reason, I feel like I've dodged a lot of bullets.

As I commented in a private email to Greg Comeau a while back, I've
observed that each group has its own particular rhythms and "rules"
(formal or otherwise, e.g. the topic of this sub-thread). At times,
discerning those rhythms feels like trying to see the picture hidden
within one of those stereo grams (I've never yet managed to see
one...)
-leor


Leor Zolman
BD Software
(e-mail address removed)
www.bdsoft.com -- On-Site Training in C/C++, Java, Perl & Unix
C++ users: Download BD Software's free STL Error Message
Decryptor at www.bdsoft.com/tools/stlfilt.html

 

[Christian Bau]

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);
if(t != NULL)
{
char *u;
strcpy(t, s);
u = t;
while(*u)
{
if(*u == '.')
{
*u = '_';
}
++u;
}
}
return t;
}


Is there any way to improve the code above?

Yes.

 

[Peter Nilsson]

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);

I prefer an in-place methodology or from-to as in many <string.h> functions.
It removes the need for this code to be tied to an allocation system like
malloc. [Of course, there are pros and cons to this.]

if(t != NULL)
{
char *u;
strcpy(t, s);
u = t;
while(*u)
{
if(*u == '.')
{
*u = '_';
}
++u;
}
}
return t;
}

Is there any way to improve the code above?

As others have said, the strcpy is somewhat duplicitous. If you are going to
use it though, then using strchr() in the while condition may be faster on
some implementations, if the function is commonly used with long strings
where little or no substitution is required.

Stylistically, I also prefer...

for (u = t; *u; u++)
{
...
}

....to...

*u = t;
while (*u)
{
...
++u;
}

 

[Morris Dovey]

Okay, I hear you (and Richard). Every time I select Agent's
Send Now menu option (or hit control-N, which I discovered by
accident just a few days ago is equivalent), I steel myself up
for the possibility of regretting it ;-). But after several
months of posting, for better or worse, I can state that
there's not one single response to any of my posts, even the
seriously brain-dead ones, that I ended up feeling "attacked"
over. I wonder whether my having at least a little bit of name
recognition (and certainly not to everybody) has tempered what
might otherwise have been more acerbic reactions, but for
whatever the reason, I feel like I've dodged a lot of bullets.

Good for you! I don't think there's a mean-spirited regular in
the bunch - although there's quite a range of opinion as to how
to best help people learn. The point to keep in mind is that even
when (perhaps especially when!) the criticism is particularly
sharp, it's because there's someone who's concerned about the
quality of education.

My motto (dating from my first week on comp.lang.c) has been
"Never pass up an opportunity to display your ignorance". I'll
have to admit that I had to do a Google search on your name to
understand your remark about name recognition. Rest assured that
your past accomplishments won't afford you a bit of protection
here! You'll still need to get it right.

One of my all-time favorite quips was in a response from Richard
Heathfield to a post from Dennis Ritchie in which Richard asked:
"And your C question was?". ROFLMAO every time I recall it. 8-D

As I commented in a private email to Greg Comeau a while back,
I've observed that each group has its own particular rhythms
and "rules" (formal or otherwise, e.g. the topic of this
sub-thread). At times, discerning those rhythms feels like
trying to see the picture hidden within one of those stereo
grams (I've never yet managed to see one...)

And this is, of course, exactly why we /strongly/ encourage
newcomers to lurk for a while. It really does help to take time
to puzzle out what all these strange people are really like
before engaging them in discussion of what they know they know
really well. That little bit of preparation can prevent a lot of
discomfort...

I've never been able to see the hidden pictures, too - that must
be another of my hidden talents. (-:

 

[Peter Nilsson]

[Context reinserted from previous posts...]

No, but de facto faux pas it is, nevertheless.

I just find it humorously ironic that an _uncasted_ malloc should be called
on the issue of failing to provide a declaration for malloc! <g>

I can't recall if Dan Pop's current position is that...

void *malloc();

....is sufficient when the supplied parameter on calling is of type size_t.
[I still think it isn't sufficient.]

 

[pete]

char *dot_to_underscore(const char *s)
{
char *t = malloc(strlen(s) + 1);
if(t != NULL)
{
char *u;
strcpy(t, s);
u = t;
while(*u)
{
if(*u == '.')
{
*u = '_';
}
++u;
}
}
return t;
}

Is there any way to improve the code above?

I prefer to supply the target array from the calling function.
That way the function is more versatile, it can be used
with an automatic array as well as a mallocated one.
Also, it makes it more obvious that the calling function
has the responsibility of doing any freeing, if needed.

char *dot_to_underscore(char *s1, const char *s2)
{
char *const p1 = s1;

do {
*s1++ = (*s2 == '.' ? '-' : *s2);
} while (*s2++ != '\0');
return p1;
}

 

[CBFalconer]

It would probably be much improved by including stdlib.h - I'd
probably have written: ^^^^^^^^

Harrumph - ITYM also string.h. How could that consumate expert
Trollsdale make such beginners errors?

#include <stdio.h>
char *dot_to_underscore(const char *s)
{ char c,*t=s,*u;
if (s && (t = u = malloc(strlen(s) + 1)))
{ while (c = *s++)
{ if (c == '.') c = '_';
*u++ = c;
}

*u = '\0'; /* <-------- */

}
return t;
}

but there's no way to be sure of any real improvement in either
size or performance.

You don't consider the fact that yours works and is readable an
improvement? Woops - see above addition.

 

[CBFalconer]

Is there some sort of guideline in this group that makes posting
a free-standing, self-explanatory little function using totally
standard lib calls, without the requisite #includes, into a major
faux pas? ... snip ...

Yes, by tradition. Especially honored for ERT. One learns to
guard ones rear from the attack dogs by including such caviling
clauses as "assuming suitable includes" etc.

 

[Morris Dovey]

You don't consider the fact that yours works and is readable an
improvement? Woops - see above addition.

Woops, indeed.

One more time, to see if I can get the header files right, add
the string terminator, and remove some of the clutter (braces)
that interferes with readability (-:

#include <stdlib.h>
#include <string.h>
char *dot_to_underscore(const char *s)
{ char *t=s,*u;
if (s && (t = u = malloc(strlen(s) + 1)))
do *u++ = (*s == '.') ? '_' : *s;
while (*s++);
return t;
}