Remove all occurrences of character from string

[Scotty33]

I'm new to C but not programming. I've done this in java before and it couldn't have been easier.

I'm not sure what I don't get about C Strings thats messing this up for me. I've tried a few different ways but I feel this is closest to being correct yet doesn't run.

Also the original string in this is not suppose to be modified so do I need to create a new variable using strcpy first before sending it to the function?

#include <stdio.h>
#include <stdlib.h>
#include <strings.h>


int main (int argc, const char * argv[]) {
	
	
	char* str = malloc(100); 
	char* removeChar(char* s, char c);
	char remove;


	printf("Enter a string\n");
	scanf("%s", str);
	printf("You entered %s \n", str);


	
	remove = 'o';


	removeChar(str, remove);
	
	printf("New Sting: %s\n", str);
}


char* removeChar(char* str, char c){
	
	char *dst;
	int i;
	
	for ( i = 0; i < strlen(str) && *str != 0; i++, str++ ){
        if ( *str != c )
            *dst++ = *str;
    }
	
	return *str;
}

Thanks!

[laserlight]

Also the original string in this is not suppose to be modified so do I need to create a new variable using strcpy first before sending it to the function?

Yes, since you want to operate on a copy of the original string. Note that strcpy alone is not good enough; you need to allocate space for the new string.

Also, remember to free what you malloc. Since you are using malloc(100), you might want to consider just using a char[100] instead.

[Salem]

> i < strlen(str) && *str != 0
This is a redundant test, since both become false at the same time.
It's easiest if you just test *str to detect the end of the string, since you're incrementing the pointer anyway.

The bit you're missing is *dst = '\0' to mark the end of the new string.

[Scotty33]

I updated my code but still no luck.
When I run it I can put in the string then it fails...

Enter a stringthis has zero os in it
You entered this 
Segmentation fault
logout

#include <stdio.h>
#include <stdlib.h>
#include <strings.h>


int main (int argc, const char * argv[]) {
	
	
	char* str = malloc(100); 
	char* removeChar(char* s, char c);
	char remove;
	char* str2 = malloc(strlen(str) + 1);


	printf("Enter a string\n");
	scanf("%s", str);
	printf("You entered %s \n", str);
	
	remove = 'o';


	removeChar(str, 'o');
	
	printf("New Sting: %s\n", str);
	
	free(str);
	free(str2);
	free(removeChar);
}


char* removeChar(char* str, char c){
	
	char *dst;
	
	while (*str != 0) {
        if ( *str != c )
            *dst++ = *str;
    }
	
	*dst = '\0';
	
	return *str;
}

When I compile it I get one warning "warning: return makes pointer from integer without a cast"

I guess I'm not getting what I'm doing wrong, is it in the pointers?

[laserlight]

This:

#include <strings.h>

should be:

#include <string.h>

You should not be calling strlen(str) until after you have read into str.

This does not make sense because removeChar is a function:

free(removeChar);

I suggest that you move the forward declaration of removeChar to before the definition of the main function.

[hk_mp5kpdw]

char* removeChar(char* str, char c){
	
    char *dst;
	
    while (*str != 0) {
        if ( *str != c )
            *dst++ = *str;
    }
	
    *dst = '\0';
	
    return *str;
}

1. dst does not point anywhere valid. At the start of the function, the value (address) in the area occupied by the pointer is random. When you start accessing this location, you are writing to a random location. Do you mean to set it to point to str?
2. Note that you are not incrementing str at all in that loop.
3. The function is supposed to return a char* value. *str is a single character. str is a char*. So, which one would you need to be returning? This is what is causing your warning "return makes pointer from integer without a cast" since it is attempting to take a char and make it a pointer to char during the return.

[Adak]

A different take on it, only superficially tested.

int main(void) {

   int i=0, gap;
   char myarray[80];

   scanf("%[^\n]",myarray);
   while (myarray[i]!='\0') {
	   printf("\n text %3d: %c",i,myarray[i]);
	   ++i;
   }
   printf("\n\n");
   gap=i=0;
   while(myarray[i]) {
      if(myarray[i]== 'o') {
         gap++;
         ++i;
      }
      myarray[i-gap] = myarray[i];
      ++i;
   
   }
   myarray[i-gap]='\0';
   printf("%s", myarray);
   putchar('\n');
   return 0;

Uses the code from the last reply (scanf() is lousy for user input), but the shifting of the letters is there.

[Scotty33]

I got it working, thanks for the help guys!

Just a few dumb logic / syntax errors and its all good.

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


void delChar(char* s, char c);	


int main (){
	
	
	char* str = malloc(100); 




	printf("Enter a string\n");
	fgets(str,100,stdin);
	printf("You entered %s\n", str);
	
	char* str2 = malloc(strlen(str) +1);
	strcpy(str2, str);
	
	printf("String 1 Copy: %s\n", str2);
		
	delChar(str2, 'o');
	
	printf("New String: %s\n", str2);
	
	free(str);
	free(str2);
	
	return 0;
}


void delChar(char* str, char c){
	
	printf("You are in removeChar\n");
	int index;
	int length = strlen(str);
		
	for(index = 0; index < length; index++){
		if(c == str[index]){
			for (int i = index; i < length - 1; i++) {
				str[i] = str[i+1];
			}
			length--;
			index--;
		}
	}
}

[iceaway]

If you want to optimize your delChar-function, you could create a new string and only copy the valid characters to it, instead of shifting the entire original string on every occurrence of character to remove.

void delChar(const char *, char *, char);  
  
int main ()
{
    char str[100], str2[100]; /* No need to malloc the memory in this case */
 
    printf("Enter a string\n");
    fgets(str,100,stdin);
    printf("You entered %s\n", str);
         
    delChar(str, str2, 'o');
     
    printf("New String: %s\nOld string: %s\n", str2, str);
  
    return 0;
}


void delChar(const char *src, char *dst, char c)
{    
    while (*(src) != '\0') {
        if (*(src) != c)
            *(dst++) = *(src);    
        src++;
    }
    *(dst) = '\0';
}

[laserlight]

If you want to optimize your delChar-function, you could create a new string and only copy the valid characters to it, instead of shifting the entire original string on every occurrence of character to remove.

Actually, only the remaining substring is shifted, but yes, it can be expensive. However, it is possible to perform the deletion efficiently in-place. I believe Adak's post #7 demonstrates this, though I find it easier to maintain two pointers: one for the current destination, the other for the current source character that is not to be deleted. The loop terminates when the second pointer reaches the end of the string, upon which a null character is written at where the destination pointer points to.

EDIT:
Looking carefully at iceaway's example, I think it implements precisely what I described, except that iceaway was not aware that the algorithm would work in-place.

[Adak]

@iceaway:

In my program, (#7), there is no shuffle of the char's every time that a char needs to be deleted.

Look closely, and you'll see that it goes through the string just ONCE, and moves each char that needs to be moved, to it's final position, in a single move.

I believe Laserlight's idea would be the most efficient, since it does no subtraction (like mine does), and is in place. Your program and mine would be VERY close. Naive programs that shuffle the entire string every time another char is deleted, would be a woeful last place.

[laserlight]

There is no algorithm which is more efficient. Although an indexed auxiliary array would be about even, as would your algorithm.

iceaway's algorithm is the same as yours. It is merely a different implementation that was used with a separate destination array instead of in-place.

Looking at Scotty33's code from post #4, I think Scotty33 was very much on track. Compare this:

char* removeChar(char* str, char c){
     
    char *dst;
     
    while (*str != 0) {
        if ( *str != c )
            *dst++ = *str;
    }
     
    *dst = '\0';
     
    return *str;
}

with a corrected version:

char *removeChar(char *str, char c) {
    const char *src = str;
    char *dst = str;

    while (*src != '\0') {
        if (*src != c) {
            *dst++ = *src;
        }
        ++src;
    }
    *dst = '\0';

    return str;
}

Oh, but since the original string was not supposed to be modified anyway, iceaway's use of a separate array is certainly not wrong.

[Adak]

That's why I changed my post. Since Scotty33 said he was copying the string first, I didn't worry about changing one of them.

Copying only the chars to be kept into another array, streamlines the whole job, removing one separate copy completely. Iceaway takes the lead!

Yes, Scotty was close. His algorithm is also a smart one.

[iceaway]

As you already stated, I took into account the fact that the original string should not be modified, which eliminated the method of doing it in-place. Interesting discussion nevertheless, I have an unhealthy passion for learning how to do the same thing in many different ways :-)

[Adak]

Yes, very smartly done. You da winner!

Great attitude to take.