0

I recently started cs50 and I finished caesar after great difficulty. Everything is working when I run check50 except there is a segmentation error when the input is null. I did search a few forums and similar questions here but I didn't understand the answer's given before because they all seem to refer to initializing argv without checking for argc. I don't think I have the same problem here and some would be greatly appreciated. Also, excuse my code, It is too big and I will make it shorter sometime later.

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

int main(int argc, string argv[])
{
    // using atoi to convert string argv[1] to int
    int i, j, y, z, x = atoi(argv[1]);
    string input;
    if(argc != 2)
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
    else if (argc == 2)
    {
        for (i = 0; i < strlen(argv[1]); i++)
        {
            // check if argc is an integer
            if ((int) argv[1][i] < 48 || (int) argv[1][i] > 57)
            {
                printf("Usage: ./caesar key\n");
                exit(1);
            }
        }
        input = get_string("plain text: ");
        printf("ciphertext: ");
        for (j = 0; j < strlen(input); j++)
        {
            // check if input is in caps
            if (((int) input[j] >= 65) && ((int) input[j] <= 90))
            {
                y = (int) input[j] + (x % 26);
                if (y <= 90)
                {
                    printf("%c", (char) y);
                }
                if (y > 90)
                {
                    // for wrapping the alphabets
                    y = 64 + y % 90;
                    printf("%c", (char) y);
                }
            }
            // check if input is regular letters
            else if (((int) input[j] >= 97) && ((int) input[j] <= 122))
            {
                z = (int) input[j] + (x % 26);
                if (z <= 122)
                {
                    printf("%c", (char) z);
                }
                if (z > 122)
                {
                    // for wrapping the alphabets
                    z = 96 + z % 122;
                    printf("%c", (char) z);
                }
            }
            else if (" ")
            {
                printf("%c", input[j]);
            }
        }
        printf("\n");
    }
    return 0;
}
0

Pretty straightforward. Yes, you do have the same problem. It's very common for new programmers. Look at the initial code in the program (I added the comments for clarity):

int i, j, y, z, x = atoi(argv[1]);  //line a
    string input;
    if(argc != 2)                   //line b

Line a tries to do something with argv[1]. If input is null, argv[1] doesn't exist and argc = 1, not 2. If argv[1] doesn't exist, line a will result in a seg fault - always!

So, you absolutely MUST check the value of argc first! Line b (and the code block that accompanies it) must be done first.

Programming note: The else clause is not needed. "else if" is even more unnecessary.

Let's tackle that first. When the if clause tests if something is true, and there is an else clause, there is no need to test if the same thing is false. It simply must be. THat's why an "else if" clause isn't needed.

Second, the if clause code block terminates the program if the input is bad. If the input is good, the program will simply continue. There is no need in that case to have an else clause. Further, it's a bad practice. You end up surrounding the entire remainder of the code body in an else clause that isn't needed. Worse, it can cause bugs to be introduced into code. You may forget or get confused about the surrounding else clause, or worse, when modifying code at a later date, you or another coder unfamiliar with the code might miss the else clause and modify it without realizing the nature of the else clause that starts at the beginning of the code and ends at the bottom of the code. It's just a bad practice.

Here's a tip for else and else if clauses. If code execution actually requires between choosing between two paths of code then an else clause or multiple else and else if clauses are probably in order. BUT, if its about whether one thing is true or not, and then the following code must be run (or the program terminates), then an else clause isn't necessary or appropriate.

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

1
  • Thanks for the quick reply and the answer is amazing. You addressed everything and I will definitely change my approach towards if and else function. It's a bad habit that I picked up while trying to understand some random solutions. Thanks a ton! Oct 12 '20 at 18:18

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .