CS50 Problem Set 2 Code Not Fully Executing

Question

I am trying to get the code to check that: 1) Only a numerical argument is passed through 2) If a non-numerical, number is pass through, an error message is revealed to the user.

Here are the wording of the problem statement:

Modify caesar.c at right such that instead of printing out the command-line argument provided, your program instead checks to make sure that each character of that command line argument is a decimal digit (i.e., 0, 1, 2, etc.) and, if any of them are not, terminates (with a return code of 1) after printing the message Usage: ./caesar key

Once saved, print out the integer, as via %i with printf. So, for example, the behavior might look like this:

Option 1
$ ./caesar 20

Success

20

or
**

Option 2

$ ./caesar 20x

Usage: ./caesar key

Here is the issue. Regardless of whether i input a number(20), or a number-digit ( 20x), I get a successful response, which shouldn't be the case. In fact, I should be reprompted for Usage:./caesar key when I input "20x"

Here is my line of code, and I really appreciate any and all help that I can get :

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


int main(int argc, string argv[])

{   
    if (argc == 2)
    {
        printf("Success\n%s\n", argv[1]);
        return 0;
    }
    else
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }   

    string s = argv[1];  

    for (int i=0, n = strlen(s); i < n ; i++)  
    {
        if(isdigit(s[i]))
        {
            continue;
            printf("test");
        }
        else
        {
            printf("Usage: ./caesar key \n");
            return 1;
        }    
    }
}

Answer 1

The code will never get past the test for argc == 2. The test is an if/else structure with a return statement in both clauses. That means that, no matter what, a return statement will terminate the program. Specifically, the return 0; statement will terminate the program when you want it to keep running.

Side note: Let's talk a little about efficient code. Look at the following:

    if(isdigit(s[i]))
    {
        continue;
        printf("test");
    }
    else
    {
        printf("Usage: ./caesar key \n");
        return 1;
    }    

First, the printf statement is what is called "dead code", code that never executes and can be removed without changing the program. It's dead because the continue statement will always cause the code to skip anything inside the loop that follows it, and return to the beginning of the loop. In this case, the continue statement is the actual problem and should be removed.

But, there's a bigger issue than the continue statement. In this case, the entire if/else code block can be rewritten in a simpler form.

    if(!isdigit(s[i]))
    {
        printf("Usage: ./caesar key \n");
        return 1;
    }    

We only want to exit the code if a char is NOT a digit, so simply adding the not operator, !, to the if statement negates or reverses the test. Now, if anything is NOT a digit, the exit code will execute. This is far more efficient than the original version.

When writing code that involves a test, it's always best to ask if the test should be the positive or the negative of what you're looking for? Or, for a range of values, should the test be for a number within a range or outside a range?

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)