Removing consecutive repeated characters from string using C

Question

I'm trying to remove consecutive repeated characters from a given string.

Example:

bssdffFdcrrrtttii ***#

output is supposed to be:

bsdfFdcrti *#

This code doesn't work and only prints the first char (b), I want to learn about my mistake. when I'm doing a printf test, it works but not for spaces. I think the problem might be with the new char array.

void Ex6() {
    char* string[80];
    scanf("%s", &string);
    puts(removeDup(string));
}

char* removeDup(char *string) {
    int i, c = 0;
    char* newString[80];
    for (i = 0; i < strlen(string); i++) {
        if (string[i] != string[i + 1]) {
            newString[c++] = string[i];
        }
    }
    return newString;
}

What difference do you recognize between char* string[80]; and char string[80];? "it works but not for spaces" --> Change char* string[80]; scanf("%s", &string); to char string[80]; fgets(string, sizeof string, stdin); and enable all compiler warnings to save time. Report the warnings that are unclear to you. — chux
– chux, Commented May 10, 2018 at 18:31
There's a huge difference between char* x, char* x[n] and char x[n], so please be very careful when specifying types. — tadman
– tadman, Commented May 10, 2018 at 18:40

tbrk · Accepted Answer · 2018-05-11 07:38:57Z

3

There are several problems with your program:

The declaration of newString should be char newString[80], i.e., an array of characters and not an array of pointers-to-characters, and likewise for the declaration in Ex6.
The call to scanf should then be scanf("%s", string), since string is already the address of an array of characters, but...
Use fgets to read a string from the user to ensure that you read whitespace, if it's important, and that the buffer is not exceeded.
newString is allocated on the stack and so should not be returned to the caller. It is better to do a char *newString = strdup(string), or, slightly less sloppy, char *newString = malloc(strlen(string)+1), which will call malloc for a block of memory sufficient to hold the original string, and thus the version without duplicates -- the comments rightly point out that this could be optimized. In principle, the caller, i.e., Ex6, must free the returned pointer to avoid a memory leak but it hardly matters in such a short program.
The result needs a null terminator: newString[c] = '\0'.

Otherwise, the removeDup function seems to work correctly.

So, putting all of that together:

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

char* removeDup(const char *string)
{
    size_t i, c = 0;
    size_t string_len = strlen(string);
    char *newString = malloc(string_len + 1);

    for (i = 0; i < string_len; i++) {
        if (string[i] != string[i + 1]) {
            newString[c++] = string[i];
        }
    }
    newString[c] = '\0';

    return newString;
}

#define MAX_STRING_LEN 80

void Ex6() {
    char string[MAX_STRING_LEN];
    char* result;

    if (fgets(string, MAX_STRING_LEN, stdin) != NULL) {
        result = removeDup(string);

        printf("%s", result);
        free(result);
    }
}

Finally, I agree with @tadman's comment. Since the input string must anyway be traversed to calculate the length, we may as well optimize the size of the result string:

char* removeDup(const char *string)
{
    size_t i, c = 0;
    char *newString;

    for (i = 0; string[i] != '\0'; i++)
        c += (string[i] != string[i + 1]);

    newString = malloc(c + 1);

    for (i = c = 0; string[i] != '\0'; i++) {
        if (string[i] != string[i + 1]) {
            newString[c++] = string[i];
        }
    }
    newString[c] = '\0';

    return newString;
}

edited May 11, 2018 at 7:38

answered May 10, 2018 at 18:35

tbrk

1,3081 gold badge13 silver badges20 bronze badges

Sign up to request clarification or add additional context in comments.

9 Comments

tbrk Over a year ago

I don't get a segfault after changing the declaration of newString and compiling with -Wno-return-stack-address (even though that's not good), but I'm not using Ex6.

tadman Over a year ago

You could do one pass to compute how many letters you need, and another with a buffer allocated at the right size.

chux Over a year ago

size_t string_len is a good improvement. For consistency (and correctness), i and c should be the same type size_t as string_len.

chqrlie Over a year ago

You should test the return value of fgets() to avoid undefined behavior on an empty file. You could also const qualify the argument to removeDup: char *removeDup(const char *string). Also include <stdio.h>, <stdlib.h> and <string.h>.

tbrk Over a year ago

@tadman: you're absolutely right. I've incorporated your suggestion.

|

VHS · Accepted Answer · 2018-05-10 20:26:20Z

2

There are quite a few issues in your program. It wouldn't even compile let alone run. Also, the most problematic issue is that you are returning a pointer to a local variable from a function that ceases its scope upon completion. A simplified version of your program is as follows:

void Ex6() 
{
   char string[80];
    scanf("%s", string);
        int i, c = 0;
    char newString[80];
    for (i = 0; i < strlen(string); i++) {
        if (string[i] != string[i + 1]) {
            newString[c++] = string[i];
        }
    }
    newString[c] = '\0';
    puts(newString);
}

edited May 10, 2018 at 20:26

answered May 10, 2018 at 18:39

VHS

10.2k3 gold badges25 silver badges48 bronze badges

2 Comments

chux Over a year ago

scanf("%s", string); retains OP's problem of "it works but not for spaces"

VHS Over a year ago

@chux, thanks for your feedback. I have made corrections.

Temple · Accepted Answer · 2018-05-11 15:13:44Z

1

You can do it with O(n) time and O(1) space, by modifying existing string:

#include <stdio.h>

char* removeDup(char* input) {
        char* newTail = input, *oldTail = input;
        while (*oldTail) {
            if (*newTail == *oldTail) {
                ++oldTail;
            } else {
                *++newTail = *oldTail++;
            }
        }
    return newTail;
}

int main() {
   char string[] = "bssdffFdcrrrtttii ***#";
   char* newEnd = removeDup(string);
   char* tmp = string;
   while (tmp != newEnd) {
       printf("%c", *tmp++);
   }
   //Print the last char if string had any duplicates
   if(*tmp) {
       printf("%c", *tmp++);
   }
   return 0;
}

edited May 11, 2018 at 15:13

answered May 10, 2018 at 20:37

Temple

1,6411 gold badge13 silver badges30 bronze badges

3 Comments

tbrk Over a year ago

I agree with this solution, but it depends on the expected interface. Also, your solution is more C++ than C and it may be better to add the null terminator so that some kind of substr is not required.

Temple Over a year ago

Thanks for pointing that out, somehow I missed we are dealing with C not C++

Vitor Carvalho Over a year ago

This is one great solution, I would maybe return the first char of the newTail instead of the last, but stil very nice. Oh, also maybe add a \0 in the last non-duplicate char. Great solution, though.

Collectives™ on Stack Overflow

Removing consecutive repeated characters from string using C

3 Answers 3

9 Comments

2 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

9 Comments

2 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related