Nested if-else statements are giving me problems

Question

The code I wrote is supposed to count the amount of a's, c's, t's, and g's in a char pointer list. Then if the entered char is not an a, c, t, or g, then it is supposed to add the invalid letter into the char list invalidBase. Instead when I enter the data agtcpoop it prints out Invalid Base: but the chars are not there, when it is supposed to print out poop. Please help! Here is the code:

void countBase(char *p)
{
   int aCount = 0, cCount = 0, tCount = 0, gCount = 0;
   char invalidBase[100];
   int i, j=0;
   while(*p != '\0')
{
  if(*p == 'A' || *p == 'a')
  {
     aCount++;
  }
  else if(*p == 'C' || *p == 'c')
  {
     cCount++;
  }
  else if(*p == 'T' || *p == 't')
  {
     tCount++;
  }
  else if(*p == 'G' || *p == 'g')
  {
     gCount++;
  }
  else
  {
     invalidBase[j] = *p;
  }
  j++;
  p++;
}

for(i = 0; invalidBase[i] != '\0'; i++)
{
   printf("Invalid Base: %c\n", invalidBase[i]);
}

printf(" A: %i\n C: %i\n T: %i\n G: %i\n", aCount, cCount, tCount, gCount);
}

Answer 1

Yoy invoked undefined behavior by using values of uninitialized variables having automatic storage duration, which is indeterminate.

To improve:

• Increment the counter j only if a data is stored to invalidBase.
• Use j to determine the length to print.

Improved code:

void countBase(char *p)
{
   int aCount = 0, cCount = 0, tCount = 0, gCount = 0;
   char invalidBase[100];
   int i, j=0;
   while(*p != '\0')
   {
      if(*p == 'A' || *p == 'a')
      {
         aCount++;
      }
      else if(*p == 'C' || *p == 'c')
      {
         cCount++;
      }
      else if(*p == 'T' || *p == 't')
      {
         tCount++;
      }
      else if(*p == 'G' || *p == 'g')
      {
         gCount++;
      }
      else
      {
         if(j < (int)(sizeof(invalidBase) / sizeof(*invalidBase))) /* avoid buffer overrun */
         {
            invalidBase[j] = *p;
            j++;
         }
      }
      p++;
   }

   for(i = 0; i < j; i++)
   {
      printf("Invalid Base: %c\n", invalidBase[i]);
   }

   printf(" A: %i\n C: %i\n T: %i\n G: %i\n", aCount, cCount, tCount, gCount);
}

Note that you can use switch statement instead of these many if-else statements.

void countBase(char *p)
{
   int aCount = 0, cCount = 0, tCount = 0, gCount = 0;
   char invalidBase[100];
   int i, j=0;
   while(*p != '\0')
   {
      switch(*p)
      {
      case 'A':
      case 'a':
         aCount++;
         break;
      case 'C':
      case 'c':
         cCount++;
         break;
      case 'T':
      case 't':
         tCount++;
         break;
      case 'G':
      case 'g':
         gCount++;
         break;
      default:
         if(j < (int)(sizeof(invalidBase) / sizeof(*invalidBase))) /* avoid buffer overrun */
         {
            invalidBase[j] = *p;
            j++;
         }
         break;
      }
      p++;
   }

   for(i = 0; i < j; i++)
   {
      printf("Invalid Base: %c\n", invalidBase[i]);
   }

   printf(" A: %i\n C: %i\n T: %i\n G: %i\n", aCount, cCount, tCount, gCount);
}

Answer 2

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

void countBase(char *p) {
    int aCount = 0, cCount = 0, tCount = 0, gCount = 0;
    char invalidBase[100];
    int i, j=0;
    while(*p != '\0') {
        if(tolower(*p) == 'a') {
            aCount++;
        }
        else if(tolower(*p) == 'c') {
            cCount++;
        }
        else if(tolower(*p) == 't') {
            tCount++;
        }
        else if(tolower(*p) == 'g') {
            gCount++;
        }
        else {
            // This is the correct place to increment j.
            invalidBase[j++] = *p;
        }

        // The following line is wrong. You can only increment j if an invalid
        // base was found and inserted on the array.
        // j++;
        p++;
    }

    // Your code lacked the "end of string" delimitation.
    invalidBase[j] = '\0';

    for(i = 0; invalidBase[i] != '\0'; i++) {
        printf("Invalid Base: %c\n", invalidBase[i]);
    }

    printf(" A: %i\n C: %i\n T: %i\n G: %i\n", aCount, cCount, tCount, gCount);
}

int main(){
    printf("Testing '%s'\n", "agtcpoop");
    countBase("agtcpoop");
    puts("***********************************");
    printf("Testing '%s'\n", "a");
    countBase("a");
    puts("***********************************");
    printf("Testing '%s'\n", "");
    countBase("");
    puts("***********************************");
    printf("Testing '%s'\n", "p");
    countBase("p");
    return 0;
}