Searching char array for char

Question

I have been attempting to iterate through a predetermined array of characters and compare it to a scanned in single char. If the scanned char is in the array I want to add it into a 2d array if it isn't in the array I want error handling.

My code is currently

    char c;
    char legalChar[] = "./\\=@ABCDEFGHIJKLMNOPQRSTUVWXYZ\n";
    int rowCount = 0;
    int colCOunt = 0;
    int i = 0;
    FILE * map;

    while (c = fgetc(map), c != EOF) {
        while (i < (sizeof(legalChar))){
            if (c == legalChar[i]){
                if (c == '\n'){
                    /*Add 1 to number of rows and start counting columns again */
                    rowCount++;
                    colCount = 0;
                }
                else {
                    /*would have code to add char to 2d array here */
                    colCount++;
                }
            }
        i++;
    }

I planned to have

    if (c != legalChar[i]){
        /*Error handling */
    }

but this doesn't work because it just jumps into this if statement on every iteration.

The output from the program at the moment is colCount gets assigned 1 and rowCount stays at 0. All chars that are iterated through are inside of the legalChar[] array so I'm not sure what I'm doing wrong.

Any advice would be greatly appreciated.

Thanks

`EOF` is never equal to a `char`, define `c` as `int` instead. — Yu Hao, Aug 08 '13 at 03:37
You can use `if (strchr(legalChar, c)) { /* char is legal */ } else { /* char is not legal */ }` to test the character without writing the loop yourself. — caf, Aug 08 '13 at 04:17
Did you try to compile your code ? : `error C2065: 'colCount': undeclared identifier` Mistake with wrong variable name. — boleto, Aug 08 '13 at 04:44
@YuHao: The advice to use `int` rather than `char` is correct; the reasoning is more nuanced than '`EOF` is never equal to a `char`'. See, amongst many other possibilities, [`while != EOF loop won't execute](http://stackoverflow.com/questions/13694394/while-eof-loop-wont-execute/13694450#13694450). — Jonathan Leffler, Aug 08 '13 at 05:09
@PHIfounder I should say that always use an `int` to compare with `EOF`, see Jonathan's comment and link. — Yu Hao, Aug 08 '13 at 06:27
@YuHao It is not correct to say that `EOF` is never equal to a `char`, what `EOF` can never be equal to is any `unsigned char`. The `EOF` macro is defined by the standard only as being some negative number, and most implementations give it the value `-1`, which definitely fits into a `char`. — This isn't my real name, Aug 08 '13 at 20:43
@ElchononEdelson See @Jonathan's comment and linked answer. My saying is incorrect, but so is yours. Native `char` can be implemented as either `signed char` or `unsigned char`. — Yu Hao, Aug 09 '13 at 00:35
@YuHao Native char can be signed, therefore EOF can be equal to `(char)-1`. Every function that returns either a valid character value or EOF always specifies that the character value is an int converted to `unsigned char`. EOF being a negative value, it can never equal an unsigned char. — This isn't my real name, Aug 09 '13 at 14:58
@ElchononEdelson My point is in some implementations, `char` may be unsigned, so it's wrong to say that `EOF` always fits into a `char. — Yu Hao, Aug 09 '13 at 15:03

Matt Bryant · Answer 1 · 2013-08-08T04:43:14.167

Your problem is that if (c != legalChar[i]) is almost always true. Say the character entered is M, which is clearly in legalChar. If you check that c != legalChar[i], you're checking that c != '.' the first time, which is clearly true.

The better way to handle this is by having a flag value that starts false and is set to true if you find something. Once you finish the loop, if the flag is still false, then you know the value wasn't found.

Additionally, you should reset i each time you go through the loop, and a for loop makes more sense than a while loop, especially if you're using c99, as i can be declared within the loop..

int c;
char legalChar[] = "./\\=@ABCDEFGHIJKLMNOPQRSTUVWXYZ\n";
int rowCount = 0;
int colCOunt = 0;
int i = 0;
int found = 0;
FILE * map;

while (c = fgetc(map), c != EOF) {
    found = 0;
    for (i = 0; i < sizeof(legalChar); i++){
        if (c == legalChar[i]){
            if (c == '\n'){
                /*Add 1 to number of rows and start counting columns again */
                rowCount++;
                colCount = 0;
            }
            else {
                /*would have code to add char to 2d array here */
                colCount++;
            }
            found = 1;
            // break out of loop here?
        }
    }
    if (!found) {
        // Error handling here
    }
}

The same to you: `EOF` is never equal to a `char`, define `c` as `int` instead. — Yu Hao, Aug 08 '13 at 04:41
Thanks for that Matt. I just couldn't get my head around how to break out of the loop correctly when I found an illegal char with my original code. Works great! — user2662961, Aug 08 '13 at 06:40

score 1 · Answer 2 · answered Aug 08 '13 at 04:52

Here I do a little test and my code work good, maybe you can try:

#include "stdio.h"
  2 int main()                                                                                                                                              
  3 {
  4     char c;
  5     char legalChar[33] = "./\\=@ABCDEFGHIJKLMNOPQRSTUVWXYZ\n";
  6     int rowCount = 0;
  7     int colCount = 0;
  8     int i = 0;
  9     FILE * map;
 10     if(NULL ==(map = fopen("error.txt","r")))
 11             return -1;
 12     int is_ok; 
 13     while (!feof(map))
 14     {
 15          c = fgetc(map);
 16          i = 0;
 17          is_ok = 0;
 18          while (i < (sizeof(legalChar)))
 19          {
 20             if (c == legalChar[i])
 21             {
 22                 if(c == '\n')
 23                 {
 24                     rowCount++;
 25                     colCount = 0;
 26                 }   
 27                 else
 28                 {
 29                     colCount++;
 30                 }   
 31                 is_ok = 1;
 32             }   
 33             else
 34             {
 35                 if(i == 31&&is_ok == 0)// not the char in legalChar
 36                 {
 37                     printf("This char %c is ilegal in %d ,%d \n",c,rowCount,colCount);
 38                     colCount++;
 39                 }  
 40             }
 41             ++i;
 42         }
 43     }  
 44     return 0;
 45 }

Why check `if(i == 31&&is_ok == 0)` every loop? – Matt Bryant Aug 08 '13 at 04:59 — Matt Bryant, Aug 08 '13 at 04:59
@MattBryant To see if c is in legalChar[33] or not. – Lidong Guo Aug 08 '13 at 05:31 — Lidong Guo, Aug 08 '13 at 05:31

score 1 · Answer 3 · answered Aug 08 '13 at 04:54

I think you can simplify this code by using strchr:

    char c;
    char legalChar[] = "./\\=@ABCDEFGHIJKLMNOPQRSTUVWXYZ\n";
    FILE * map;
    int legal = 1;

    while (c = fgetc(map) && c != EOF && 1 == legal) {
        if (NULL == strchr(legalChar, c)) {
           legal = 0;
           // Error message pointing out invalid character
        }
        else {
           // Add to array
        }
    }

The above will abort the loop once an invalid character is found. If you don't want to do that, just remove all references to the "legal" variable.

I originally tried to use strchr but on compiling I would get an error suggesting I didn't have #include even though I had it in my code. So I decided to just write my own code for it. Thanks for the help though :) — user2662961, Aug 08 '13 at 06:41

Searching char array for char

3 Answers3