2

I have a function that counts the number of unique characters in a 2D array by looping over it and increasing the count in each cell of a 1D array by 1 each time a character from the valid char array is found. I then loop over the 1D array and each time a cell with a number higher than 0 is found, a counter is increased. If this number is higher than the height/width of my structure, it returns false.

'.' represents an empty space and whilst it is valid in the scheme of the program, it should not count as a unique character.

I was wondering if there was a way to create a function with the same functionality, but much shorter.

bool uniqueChars (Bookcase *b)
{
   int i, j, chars[8] = {0}, cnt = 0;
   char validChars[10] = {"KRGYBMCW."};

   bNullPoint(b);

   for (i = 0; i < b->height; i++) {
      for (j = 0; j < b->width; j++) {
         b->shelves[i][j] = toupper(b->shelves[i][j]); /* To aid with testing*/
         if (strchr(validChars, b->shelves[i][j])) {
            if (b->shelves[i][j] == 'K') {
               chars[0] += 1;
            }
            if (b->shelves[i][j] == 'R') {
               chars[1] += 1;
            }
            if (b->shelves[i][j] == 'B') {
               chars[2] += 1;
            }
            if (b->shelves[i][j] == 'G') {
               chars[3] += 1;
            }
            if (b->shelves[i][j] == 'C') {
               chars[4] += 1;
            }
            if (b->shelves[i][j] == 'Y') {
               chars[5] += 1;
            }
            if (b->shelves[i][j] == 'W') {
               chars[6] += 1;
            }
            if (b->shelves[i][j] == 'M') {
               chars[7] += 1;
            }
         } else {
            return false;
         }
      }
   }
   for (i = 0; i < 8; i++) {
      if (chars[i] > 0) {
         cnt += 1;
      }
   }
   if (cnt > b->height) {
      return false;
   }
   return true;
}
Davospike
  • 91
  • 1
  • 9

2 Answers2

3

Declare a character array or a string literal as for example

const char *letters = "KRBGCYQM.";

and then use the standard string function strchr declared in the header <string.h> like

char *p = strchr( letters, b->shelves[i][j] );
if ( p != NULL ) 
{
    if ( b->shelves[i][j] != '.' ) ++chars[p - letters];
}
else
{
    return false;
}

Pay attention to that it is unclear for readers of your code why the character '.' is included though it is not counted.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • I kept getting stack-buffer-overflow when I ran it but that seems to be a problem in the wider program as I tested your method on a smaller scale and it worked, so thank you! – Davospike Dec 08 '20 at 15:58
0

Can I suggest bit-fields instead of the chars array? Something like this:-

present = 0
foreach char c in b->shelves
    if c is a uppercase letter
        present |= 1 << (c - 'A')
present &= valid letters bit pattern (this is a constant and is the or of 1 shifted by each letter)
return number of bits in present <= b->height

Alternatively, if you don't like that, use a switch rather than the sequence of if tests:-

switch b->shelves[i][j]
    case 'K'
        ++chars[0]
    other cases for the valid letters
        ++chars[whatever]
    default:
        error - an invalid character
Skizz
  • 69,698
  • 10
  • 71
  • 108
  • *Can I suggest bit-fields...* Why? You can't use bit-fields for counting. – Andrew Henle Dec 08 '20 at 15:56
  • We've not looked at bit-fields or the bitwise operators yet, so I might try and implement this just to seem ahead of the curve, thank you for the idea – Davospike Dec 08 '20 at 15:56
  • @AndrewHenle: Well, in the sample code, although there is a count for each letter, the only test done is for non-zero, so why count? A bit field works just as well and uses less memory (OK, a few bytes, but it will provide an extra tool in the OPs bag of tricks as the OP seems to be learning at the moment). You can then AND the field to check for invalid entries and it's quite easy to count the number of set bits. – Skizz Dec 10 '20 at 01:01