tags:

views:

346

answers:

8

Possible Duplicates:
Painless way to trim leading/trailing whitespace in C?
Trim a string in C

Hi guys, I was writing the String trim method in c and this is the code I came up with. I think it does the job of eliminating leading and trailing whitespaces however, I wish the code could be cleaner. Can you suggest improvements.

void trim(char *String)
{
int i=0;j=0;
char c,lastc;
while(String[i])
{
   c=String[i];
   if(c!=' ')
   {
     String[j]=c;
     j++;
   }
   else if(lastc!= ' ')
   {
     String[j]=c;
     j++;

   }
   lastc = c;
   i++;
}

Does this code look clean ??

+4  A: 

It doesn't look clean. Assuming the first character is a space, you're using lastc with an undefined value. You're leaving one space at the end (if there's a space at the end, when it's hit c will be a space and lastc won't).

You're also not terminating the string. Assuming you fix the uninitialized lastc problem, you'll transform " abc" to "abcbc", since it's not being shortened at any point.

The code also collapses multiple spaces inside the string. This isn't what you described; is it desired behavior?

David Thornley
+1  A: 

There's several problems with that code. It only checks for space. Not tabs or newlines. You are copying the entire non-whitespace part of the string. And you are using lastc before setting it.

Here's an alternate version (compiled but not tested):

char *trim(char *string)
{
    char *start;
    int len = strlen(string);
    int i;

    /* Find the first non whitespace char */
    for (i = 0; i < len; i++) {
        if (! isspace(string[i])) {
            break;
        }
    }

    if (i == len) {
        /* string is all whitespace */
        return NULL;
    }

    start = &string[i];

    /* Remove trailing white space */
    for (i = len; i > 0; i--) {
        if (isspace(string[i])) {
            string[i] = '\0';
        } else {
            break;
        }
    }

    return start;
}
JayM
A: 

Instead of comparing a character with the space character ' ', I'd use the "isspace" function, which I believe is defined in ctype.h.

Tom
A: 

I don't know about clean, but I find it hard to follow. If I needed to do this I'd initially think of it in two phases:

  1. Figure out how many characters to drop from the beginning, then memmove the rest of the string (including the null terminator) to the start address. (You might not need the memmove if you are allowed to return a different start pointer, but if so you need to be very careful with memory hygiene.)
  2. Figure out how many characters to drop from the (new) end and set a new null terminator there.

I might then look more closely at a one-pass solution like you seem to be trying to implement, but only if there was a speed problem.

By the way, you probably want to use isspace() rather than checking only for space.

crazyscot
+1  A: 

There are some problems: lastc could be used uninitialized. And you could make use of a for loop instead of a while loop, for example. Furthermore, trim/strip functions usually replace spaces, tabs and newlines.

Here's a solution using pointers that I wrote quite a while ago:

void trim(char *str)
{
    char *ptr = str;
    while(*ptr == ' ' || *ptr == '\t' || *ptr == '\r' || *ptr == '\n') ++ptr;

    char *end = ptr;
    while(*end) ++end;

    if(end > ptr)
    {
        for(--end; end >= ptr && (*end == ' ' || *end == '\t' || *end == '\r' || *end == '\n'); --end);
    }

    memmove(str, ptr, end-ptr);
    str[end-ptr] = 0;
} 
AndiDog
+1  A: 

Here is my solution.

Short, simple, clean, commented, and lightly tested.

It uses the "isspace" classification function, so you can easily change your definition of "white space" to be trimmed.

void trim(char* String)
{
    int dest;
    int src=0;
    int len = strlen(String);

    // Advance src to the first non-whitespace character.
    while(isspace(String[src])) src++;

    // Copy the string to the "front" of the buffer
    for(dest=0; src<len; dest++, src++) 
    {
        String[dest] = String[src];
    }

    // Working backwards, set all trailing spaces to NULL.
    for(dest=len-1; isspace(String[dest]); --dest)
    {
        String[dest] = '\0';
    }
}
abelenky
This could be dangerous when strlen(String) is 0.
+1  A: 

It often makes your code more readable if you make judicious use of the standard library functions - for example, isspace() and memmove() are particularly useful here:

#include <string.h>
#include <ctype.h>

void trim(char *str)
{
    char *start, *end;

    /* Find first non-whitespace */
    for (start = str; *start; start++)
    {
        if (!isspace((unsigned char)start[0]))
            break;
    }

    /* Find start of last all-whitespace */
    for (end = start + strlen(start); end > start + 1; end--)
    {
        if (!isspace((unsigned char)end[-1]))
            break;
    }

    *end = 0; /* Truncate last whitespace */

    /* Shift from "start" to the beginning of the string */
    if (start > str)
        memmove(str, start, (end - start) + 1);
}
caf
A: 

Hi guys, Thanks a ton for all your help. I am a newbie in programming so this sure does help me a lot. I tried writing this new function. DO you think this is good enough?

void RemoveSpace(char *String)
{
    int i=0,y=0;
    int leading=0;

    for(i=0,y=0;String[i]!='\0';i++,y++)
    {
        String[y]=String[i];    // let us copy the current character.

        if(isspace(String[i]))  // Is the current character a space?
        {
            if(isspace(String[i+1])||String[i+1]=='\0'||leading!=1) // leading space
                y--;
        }
        else
            leading=1;
    }
    String[y]='\0';
}

Basically I wanted to do everything in one pass. Do you think this is a good code now? Original statement might have been ambiguous. I just wanted to eliminate leading trailing white spaces and replace all multiple white space with a single one.

Ciao- Please do comment on my code.

Phoenix
I must also give credit to the programmer in one of the links. I have modified his code to eliminate trailing and leading white space.
Phoenix