0

So the exercise here is to design a program that takes a string and removes all characters in that string that appear in the second string. So with the strings I've chosen below where the first string is abc and the second string is cde I want to get an output of ab rather than abc.

I've already seen a very neat way to do this squeeze function that only requires two simple for loops but I was wondering why my long winded way doesn't work.

#include<stdio.h>

void squeeze(char s1[], char s2[]);
void copy(char to[], char from[]);

int k=0;

main()
{
    char array1[4]="abc";
    char array2[4]="cde";
    squeeze(array1, array2);
    printf("%s", array1);
}

void squeeze(char s1[], char s2[])
{
    int j,i,m;
    m=j=i=0;
    char s3[1000];
    while(s1[i]!='\0')   //What I'm doing here is taking a character in the string
    {                      //and comparing it with all characters in the second string.
        copy(s3,s1);       //If it exists in the second string I'm 'deleting' that letter
        while(s2[j]!='\0') //from the first string. Then I start all over again with the
        {                 // next letter in line. Or at least that's the plan.
            if (s1[i]==s2[j])
            {
                k=1;
            }
            ++j;
        }

        if (k==1)
        {
            m=i;
            while(s3[m+1]!='\0')
            {
                s1[m]=s3[m+1];  //I'm 'deleting' the letter by pushing each character
                ++m;            //that is to the right of the deleted character to the  
            }                   //left of the array.
        }

        if(k!=1)
        {
            ++i;
        }
    }
    s1[i]='\0';
 }

void copy(char to[], char from[])
{
    int i;
    i=0;

    while(from[i]!='\0')
    {
        to[i]= from[i];
        ++i;
    }
    to[i]=='\0';
}
Peter Miehle
  • 5,984
  • 2
  • 38
  • 55

2 Answers2

2

inside your outer while you should reset "j" to zero.

if "k" bekomes 1, you do not increment "i" any more. if you run squeeze() a second time, you do not initialize "k" again.

never use global variables (or module local variables) like "k". This makes your code thread-unsafe.

Peter Miehle
  • 5,984
  • 2
  • 38
  • 55
  • 1
    +1 for the 'never use global variables' comment. It is a slight over-statement, but accurate here where `k` is only used inside the one function. Only ever make a variable accessible outside the function when it needs to be accessed from outside the function. – Jonathan Leffler Jul 21 '13 at 19:17
  • Alright, thanks! Btw I don't increment "i" because if I've 'deleted' the character, and so the next character is at the current position 'i' in the array. Is it not? – MethequalsMath Jul 21 '13 at 20:39
1

high five -- I'm reading that chapter, too. I think I've read it about 4 times, anyway I really need to learn by doing, I forget all the content within a few hours. That's why I've nearly finished all the exercises in that chapter - chapter 3 next!

Here's my solution -- the squeeze function is not compatible with the getline function (the /0' are forcing printf to not print to std out.

Compile on gcc 4.7.2 with

gcc -Wall -std=c99 -pedantic squeeze.c

#include <stdio.h>
#define LIM 100

void squeeze(char[], char[]);
int ggetline(char[], int);

int main()
{
    //the getline function is buggy; it would produce null strings.
    char line1[LIM] = "hello";
    char line2[LIM] = "hello2";
    squeeze(line1, line2);
    printf("%s %s\n", line1, line2);

return 0;

}


/* getline: reads line into s, returns length */
int ggetline(char s[], int lim)
{
    int c, i;

    for (i = 0; i < lim-1 && (c = getchar()) != EOF && c!='\n'; ++i)
        s[i] = c;
    if (c == '\n') {
        s[i] = c;
        ++i;
    }
    s[i] = '\0';
    return i;
}

void squeeze(char s1[], char s2[])
{
    int j, l, i, k;

    for (i = 0; s2[i] != '\0'; i++) {
        for (k = l = 0; s2[j] != '\0'; j++) {
            if (s1[j] != s2[i])
                s1[k++] = s1[j];
        }
        s2[k] = '\0';

    }
}

Good luck!

TheBlueCat
  • 1,147
  • 5
  • 19
  • 38