How to properly iterate through a command-line argument to check if each character is a digit in C?

Question

I am trying to iterate over the second command prompt that a user would enter to verify that each character in the array is indeed a digit.

After looking at ASCII, I know, that the characters 0-9 range from 48-57. I figured out that you could just use isdigit() but am still wondering why my loop does not work as intended.

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

int main(int argc, string argv[])
{
    if (argc != 2)
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }

    for(int i=0, n=strlen(argv[1]); i<n; i++)
    {
        if((int)argv[1][i] < 48 && (int)argv[1][i] > 57)
        {
            printf("Usage: ./caesar key\n");
            return 1;
        }
    }
}

Read [*Modern C*](https://modernc.gforge.inria.fr/), see [this C reference](https://en.cppreference.com/w/c), the documentation of your C compiler (e.g. [GCC](http://gcc.gnu.org/)...) and debugger (e.g. [GDB](https://www.gnu.org/software/gdb/)). Compile wih all warnings and debug info: `gcc -Wall -Wextra -g` with GCC. **Use your debugger**. Take inspiration from existing code on [github](https://github.com/) — Basile Starynkevitch, Nov 14 '20 at 06:33
Similar question : https://stackoverflow.com/questions/61614745/cs50-caesar-program-is-working-but-check50-says-it-isnt?rq=1 — Krishna Kanth Yenumula, Nov 14 '20 at 07:06
`argc` - Non-negative value representing the number of arguments passed to the program from the environment in which the program is run. — KaiserKatze, Nov 14 '20 at 10:57

selbie · Answer 1 · 2020-11-14T06:28:54.607

2

Instead of this:

    if((int)argv[1][i] < 48 && (int)argv[1][i] > 57)

This:

    if((int)argv[1][i] < 48 || (int)argv[1][i] > 57)

On a long enough timeline on Stack Overflow, the EBCIDIC crowd will come along to remind us all that it still exists. So instead of hardcoding ordinal values for ascii chars, use the character literal itself. It also makes the code's intent way more obvious:

    if ((argv[1][i] < '0') || (argv[1][i] > '9'))

Putting it altogether with some more cleanup:

for(int i=0, n=strlen(argv[1]); i<n; i++)
{
    char c = argv[1][i];
    if ((c < '0') || (c > '9'))
    {
        printf("Useage: ./caesar key\n");
        return 1;
    }
}

edited Nov 14 '20 at 06:28

answered Nov 14 '20 at 06:21

selbie

100,020
15
103
173

1

regardless of EBCDIC or any archaic charsets, using magic numbers like in `if((int)argv[1][i] < 48 && (int)argv[1][i] > 57)` is a bad idea. `if ((argv[1][i] < '0') || (argv[1][i] > '9'))` is much safer and easier to read – phuclv Nov 14 '20 at 08:27

score 1 · Answer 2 · answered Nov 14 '20 at 06:26

1

int main(int argc, string argv[])

String data type is not defined in C. Here you should use char* argv[].
Please see this answer: Does C have a string type?

Remaining issues have been pointed out by other users.

answered Nov 14 '20 at 06:26

Krishna Kanth Yenumula

2,533
2
14
26

score 0 · Answer 3 · answered Nov 14 '20 at 06:21

    if((int)argv[1][i] < 48 && (int)argv[1][i] > 57)
    {
        printf("Useage: ./caesar key\n");
        return 1;
    }

The check should be || not &&.

    if((int)argv[1][i] < 48 || (int)argv[1][i] > 57)
    {
        printf("Useage: ./caesar key\n");
        return 1;
    }

How to properly iterate through a command-line argument to check if each character is a digit in C?

3 Answers3