What do I need to change so cs50 Caesar only prints the correct message after i iterate over each character?

Question

I am stuck trying to iterate over each character to detect whether or not it is a digit, while only printing the answer once. The problem I'm having is I can get it to detect whether a character is a digit or not, but it prints out an answer for each number I put in until it reaches a letter. I'm looking how to get it to detect whether or not the input is a number or letter, and then make a decision on what to print out instead of printing success every time it detects a number. Feel like its something with my for loop but cant quite figure it out. Thanks.

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

int main(int argc, string argv[])
{    
    string n = argv[1];
    if (argc != 2)
    {
        printf("usage: ./caesar key\n");
        return 1;
    }
    else
    {
        for(int i = 0, length = strlen(n); i < length; i++)
            if(!isdigit(n[i]))
            {
                printf("usage: ./caesar key\n");
                return 1;
            }
            else
            {
                int convert = atoi(n);
                printf("Success\n");
                printf("%i\n", convert);
            }
    }
}

Does this answer your question? [Check if a string has only numbers in C?](https://stackoverflow.com/questions/41776955/check-if-a-string-has-only-numbers-in-c) — mch, Apr 02 '20 at 07:58
Instead of `atoi` you could use `strtol` which allows you to simplify the whole `else` part of `if (argc != 2)` to 3 or 4 lines of code. — Jabberwocky, Apr 02 '20 at 08:07

Eduardo Pascual Aseff · Accepted Answer · 2020-04-02T16:01:22.453

The problem is that inside the for you have an if-else statement... that means that one of both will always execute for every iterarion. For printing success just once, you should erase the else keyword (and for clarity the brackets) but keeping the else code.

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

int main(int argc, string argv[])
{    
    string n = argv[1];
    if (argc != 2)
    {
        printf("usage: ./caesar key\n");
        return 1;
    }
    else
    {
        for(int i = 0, length = strlen(n); i < length; i++)
        {
            if(!isdigit(n[i]))
            {
                printf("usage: ./caesar key\n");
                return 1;
            }         
        }
        int convert = atoi(n);
        printf("Success\n");
        printf("%i\n", convert);   
    }
}

@hellothere12311212 for making the code more readable, I suggest you to include the brackets of the for... I will edit it in my answer — Eduardo Pascual Aseff, Apr 02 '20 at 16:00

score 0 · Answer 2 · edited Jun 20 '20 at 09:12

0

What do I need to change so cs50 Caesar only prints the correct message after i iterate over each character?

It seems you aren't aware that you don't have to do that. The Caesar Specification says:

You can assume that, if a user does provide a command-line argument, it will be a non-negative integer (e.g., 1). No need to check that it’s indeed numeric.

edited Jun 20 '20 at 09:12

Community

1
1

answered Apr 02 '20 at 08:15

Armali

18,255
14
57
171

What do I need to change so cs50 Caesar only prints the correct message after i iterate over each character?

2 Answers2