HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Writing strcat (string concatenate) in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
concatenatestrcatwritingstring

Problem

char *cat_string(char *to_cat, char *destination) {
  char *copy = destination;
  while (*copy != '\0') {
    *copy++;
  }
  while (*to_cat != '\0') {
    *copy++ = *to_cat++;
  }
  *copy = '\0';
  return destination;
}


I would like to know if this is an efficient and effective way of writing this function. It seems silly to have to iterate through the whole array just to find the \0 character.

Also, should I be returning the pointer to the destination? What's the best practice there?

Solution

You renamed strcat() to cat_string(), so I suppose it's fitting that you reversed the traditional order of the parameters as well. ☺

I would make three changes:

  • Changing the first argument to const helps prevent users notice if they call the function with the parameters swapped (but doesn't prevent it in all cases).



  • You have a stray pointer dereference. It was misleading, but luckily harmless. (In the first while loop, you just want to advance the copy pointer. You don't care about what value it points to as you advance; you only care when you check for the NUL terminator.)



  • You can remove a statement just by changing the loop structure.



Edit (Rev 3): Previous recommendation was buggy, and has been retracted. Credit to @MarcvanLeeuwen.

To answer your questions…

  • The only way to find out where to start appending is to walk the entire destination string to locate where it ends. That's just how C-style strings work.



  • Yes, returning destination seems appropriate. It's analogous to what strcat() does.



One final note: two spaces for indentation is too stingy, in my opinion. It's insufficient for readability, and it also encourages inappropriately deep nesting (which would be a symptom of poorly organized code).

char *cat_string(const char *to_cat, char *destination) {
    char *copy = destination;
    while (*copy != '\0') {
        copy++;                 /* Removed superfluous dereference. */
    }
    …
}

Code Snippets

char *cat_string(const char *to_cat, char *destination) {
    char *copy = destination;
    while (*copy != '\0') {
        copy++;                 /* Removed superfluous dereference. */
    }
    …
}

Context

StackExchange Code Review Q#42264, answer score: 5

Revisions (0)

No revisions yet.