Thread: give a look into my TRIM function (please comment)

don't use void main either

It would be a good idea to provide an additional buffer of where you want the trimmed string stored. Modifying strings "in place" is a bit anti-social (especially in large programs where it may go unnoticed for a while).
You cannot for example trim

Code:

char *s= "   	string teste   	   ";

The function returns a pointer that may not point to the first element in the array - not a good idea at all! If the array was dynamically allocated that would certainly lead to a memory leaks. Instead, move the desired data to the beginning of the array, and be sure to return a pointer to the first element.

>What is wrong with recursion?
It doesn't really simplify the problem to use recursion, and you can easily just use two loops to do the same thing.

>How should I change it?
Use loops:

Code:

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

char *trim_str(char *string);

int main()
{
  char s[] = "   	   ";
  printf("--%s--",trim_str(s));
  return 0;
}

char *trim_str(char *string){
  char *a;

  while ( *string && isspace ( *string ) ) {
    string++;
  }
  a = string + ( strlen ( string ) - 1 );
  while ( a != string && isspace ( *a ) ) {
    a--;
  }
  if ( *a != '\0' )
    a[1] = '\0';

  return(string);
}

>and how would I change it?
String literals cannot be portably modified. To fix your code to handle them you will need to either do some dynamic memory allocation, or require the caller to pass a buffer large enough to hold the trimmed string.

i changed it. now the function do not returns a pointer. now it change the string directly on its location:

Code:

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

void trim_str(char *string);

int main(){
	char str[] = "     	string test   	";
	
	trim_str(str);
	
	printf("-%s-", str );
	
	return(0);
}


void trim_str(char *string){
	char *a;
	a = string;
	if (isspace(*string)){
		while (isspace(*a++)); /* set pointer to the first non-space char */
		a-=2;
		while (*a++ != '\0'){
			*string = *a;
			string++;
		}
		string-=(a-string); /*reset pointer to startup pos*/
	}
	string+=strlen(string)-1; /* set pointer to the last char */
	while(isspace(*string--));
	string+=2;	
	*string='\0';
}

please, comment it!

If you want a safe implementation, you should be passing a buffer to copy the info into. Otherwise, again, string literals will do BadThingsTM.

Code:

trim_str("  BadThingsTM    " );

Also, you can simplify this a bit...

Code:

void trim( char *O0OO, char *O00O )
{
        char *O000 = O0OO;
        char *OO0O = O000+strlen(O0OO)-1;
        while( isspace( *O000 ) ) O000++;
        while( isspace( *OO0O ) ) OO0O--;
        while( O000 <= OO0O )
                *O00O++ = *O000++;
}

Something like that, assuming I'm reading your problem right. However, you'll not want to turn that in for homework.

Quzah.

One thing I noticed was some unnecessary math that could be eliminated by using better structured loops. For instance:

Code:

while (isspace(*a++)); 
		a-=2;
		while (*a++ != '\0'){
			*string = *a;
			string++;
		}

Could be made more readable using something like:

Code:

while (isspace(*a)) a++; 
		while (*a != '\0'){
			*string = *a;
			string++, a++;
		}

Otherwise, the function looks good. Here's another version of the function:

Code:

 char * trim(char * str)
{
 int length = strlen(str);

     if(length > 0)
    {
     char * data = str;
     char * front = str;
     char * back = str[length];

     while(isspace(*front))  ++front;

     while(isspace(*(back-1)))  --back; 

     while(front < back)  *(data++) = *(front++);  

     *data = 0;
    } 

 return str;
}