Two functions working in isolation, but not together

Question

I have two functions rmdup and rmvow, when I run these functions in isolation they work perfectly, but when they're together in a program they don't work at all?

int rmdup ( char name[] ) 
{

  char nodup[20] = {''\0''};
  int l = strlen(name);
  int i = 0, j, k = 0;

  while ( i < l ) 
  {
    j = i + 1;

    if ( name[i] == name[j] ) 
    {
      nodup[k] = name[i];
      i+2;
      while(nodup[k] == name[i]) 
      {
        i++;
      }
      k++;
    }
    else 
    {
      i++;
    }

    nodup[k] = ''\0'';
  }

  strcpy(name, nodup);
}

The above works in isolation, as does the below.

int rmvow ( char name[]  ) 
{
  char novow[20] = {''\0''};
  int l = strlen(name);
  int i = 0, j = 0;

  while ( i < l ) 
  {
    if ( name[i] == ''a'' || 
         name[i] == ''e'' ||
         name[i] == ''i'' ||
         name[i] == ''o'' ||
         name[i] == ''u'' ) 
    {
      i++;
    }
    else 
    {
      novow[j] = name[i];
      i++;
      j++;
    }

    novow[j] = ''\0'';
  }

  strcpy(name, novow);
}

Please learn to properly indent your code. It makes it much easier to read and follow the flow of execution. You've not shown how you're trying to "use them together in a program", so how are we to be expected to explain why they don't work? — Ken White, Oct 28 '14 at 18:38
It is not clear what you might mean by *"when they're together"* or even *"they don't work at all"*. Provide the code where you are using them together, explain when each *should*, and the actual results. Your code layout is unconventional at best, and very hard to read - if you want others to read your code and help, you should make it as easy as possible. — Clifford, Oct 28 '14 at 18:39
What language is this, and why is it tagged `strcpy`? Is that relevant? What are `''\0''`, `''a''` etc.? They do not compile as C code which this otherwise appears to be. — Clifford, Oct 28 '14 at 18:49
The code is "dangerous" in that it will not work correctly for any string length. Safer to modify the string in place rather than using the fixed length temporary arrays `novow` and `nodup`. — Clifford, Oct 28 '14 at 19:16
@ElisJones : As noted in the comments the programming language use would help! I'd tag it myself, but it does not compile as C so I am not certain it is C. It is good SO citizenship to improve your question in line with comments posted. Just because you have solved your problem, as it stands the question does nothing to enhance the sum of knowledge that is SO - especially since we do not know what the problem was. — Clifford, Oct 28 '14 at 20:19

Clifford · Answer 1 · 2014-10-28T19:25:28.737

The corrected and complete code below outputs:

aabbccddeeff
abcdef
bcdf

Which seems to be correct. The only corrections were the i += 2 in rmdup(), and the valid character constants. Compiled as C code in the absence of any other information.

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

int rmdup ( char name[] ) ;
int rmvow ( char name[] ) ;

int main(void) 
{
    char test[] = "aabbccddeeff" ;

    printf( "%s\n", test ) ;
    rmdup( test ) ;

    printf( "%s\n", test ) ;
    rmvow( test ) ;

    printf( "%s\n", test ) ;

    return 0;
}

int rmdup ( char name[] ) 
{

  char nodup[20] = {'\0'};
  int l = strlen(name);
  int i = 0, j, k = 0;

  while ( i < l ) 
  {
    j = i + 1;

    if ( name[i] == name[j] ) 
    {
      nodup[k] = name[i];
      i+=2;
      while(nodup[k] == name[i]) 
      {
        i++;
      }
      k++;
    }
    else 
    {
      i++;
    }

    nodup[k] = '\0';
  }

  strcpy(name, nodup);
}

int rmvow ( char name[]  ) 
{
  char novow[20] = {'\0'};
  int l = strlen(name);
  int i = 0, j = 0;

  while ( i < l ) 
  {
    if ( name[i] == 'a' || 
         name[i] == 'e' ||
         name[i] == 'i' ||
         name[i] == 'o' ||
         name[i] == 'u' ) 
    {
      i++;
    }
    else 
    {
      novow[j] = name[i];
      i++;
      j++;
    }

    novow[j] = '\0';
  }

  strcpy(name, novow);
}

However the code is dangerous since there there is no protection against overflowing the novow or nodup arrays. Both are unnecessary; in both case you can write the data directly back to name since the destination index is always <= source index.

int rmdup ( char name[] ) 
{

  int l = strlen(name);
  int i = 0, j, k = 0;

  while ( i < l ) 
  {
    j = i + 1;

    if ( name[i] == name[j] ) 
    {
      name[k] = name[i];
      i+=2;
      while(name[k] == name[i]) 
      {
        i++;
      }
      k++;
    }
    else 
    {
      i++;
    }

    name[k] = '\0';
  }
}

int rmvow ( char name[]  ) 
{
  int l = strlen(name);
  int i = 0, j = 0;

  while ( i < l ) 
  {
    if ( name[i] == 'a' || 
         name[i] == 'e' ||
         name[i] == 'i' ||
         name[i] == 'o' ||
         name[i] == 'u' ) 
    {
      i++;
    }
    else 
    {
      name[j] = name[i];
      i++;
      j++;
    }

    name[j] = '\0';
  }
}

@ElisJones : Since you made no effort to clarify the question, I truly regret even attempting to answer to be honest. — Clifford, Oct 28 '14 at 20:21
Sorry? I was attempting to write a program, "Soundex", removed vowels and duplicates from a string and then encoded the remaining letters. For example, Clifford -> Clffrd -> Clfrd -> C1432. — Elis Jones, Nov 08 '14 at 15:22

Two functions working in isolation, but not together

1 Answers1