0

Sorry, I know this has been asked before but I have read all the answers and nothing works! Please help.

ERRORS
:) caesar.c exists.
:) caesar.c compiles.
:) encrypts "a" as "b" using 1 as key
:( encrypts "barfoo" as "yxocll" using 23 as key
    expected "ciphertext: yx...", not "ciphertext: y\..."
:( encrypts "BARFOO" as "EDUIRR" using 3 as key
    expected "ciphertext: ED...", not "ciphertext: E\..."
:( encrypts "BaRFoo" as "FeVJss" using 4 as key
    expected "ciphertext: Fe...", not "ciphertext: F\..."
:( encrypts "barfoo" as "onesbb" using 65 as key
    expected "ciphertext: on...", not "ciphertext: o\..."
:( encrypts "world, say hello!" as "iadxp, emk tqxxa!" using 12 as key
    expected "ciphertext: ia...", not "ciphertext: i\..."
:) handles lack of key
:) handles non-numeric key
:) handles too many arguments
// open main function
int main(int argc, string argv[])
{
    // don't accept more than one string for the key
    if (argc != 2)
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
    // accept key if one string
    else if (argc == 2)
    {
        // check validity of key - is it a letter or punctuation? reject if true.
        bool valid_key = true;
        int i;
        for (i = 0; i <= strlen(argv[1]); i++)
        {
            if (isdigit(argv[1][i]))
            {
                valid_key = true;
            }
            else if (isalpha(argv[1][i]) || ispunct(argv[1][i]))
            {
                valid_key = true;
                printf("Usage: ./caesar key\n");
                return 1;
            }
        }
            if (i == strlen(argv[1]))
            {
                // convert to integer
                int k = atoi(argv[1]);
                // ask for plaintext input
                string plain = get_string("plaintext: ");
                int j;
                // print ciphertext output before actually enciphering text
                printf("ciphertext: ");
                for (j = 0; j <= strlen(plain); j++)
                {
                    // if uppercase letter, retain uppercase
                    // convert from ASCII to index, encode, convert to ASCII
                    if isupper(plain[j])
                    {
                        char a = plain[j] - 'A';
                        char ci = (a + k) % 26;
                        char pi = ci + 'A';
                        printf("%c", pi);
                    }
                    else if islower(plain[j])
                    {
                        // if lowercase letter, retain lowercase
                        // convert from ASCII to index, encode, convert to ASCII
                        char b = plain[j] - 'a';
                        char ci2 = (b + k) % 26;
                        char pi2 = ci2 + 'a';
                        printf("%c", pi2);
                    }
                    else
                    {
                        // just print non-alpha characters as inputted
                        printf("%c", plain[j]);
                    }
                }
            }
       }
        // new line and exit programme
       printf("\n");
       return 0;
}
4
  • What follows after the "\..." of your current output, i.e. what is the complete error message?
    – ALL
    May 12 '20 at 16:14
  • ~/caesar/ $ ./caesar 3 plaintext: BARFOO ciphertext: EDUIRR ~/caesar/ $ May 12 '20 at 16:48
  • is that what you mean? as far as I can tell the outputs are exactly identical. There's no errors on compiling, it's only during check50 that there's a problem. May 12 '20 at 16:49
  • No, I've talked about the output on the check50-website. Below the test results in the IDE window is also a link which lead to a more detailed report (at least for the inputs and outputs - in most cases). That helped me a lot to identify the possible suspects in the code.
    – ALL
    May 13 '20 at 8:13
0

When I run this code, it never asks for the plaintext. This is because of this line:

        if (i == strlen(argv[1]))

This will not be true when the previous code completes. At the end of the loop, i has been incremented to strlen(argv[1])) + 1 for the loop to terminate. So, since this fails, none of the rest of the code will execute.

As a programming note, you're also making a common error that many new programmers make. When setting up a test for something that must be either true or false, they test for true (or false), but then surround the remaining code in the program in an else clause or retest for the false(or true) and surround the remaining code in that test. Let me give an example.

if (argc != 2)
{
    printf("Usage: ./caesar key\n");
    return 1;
}
// accept key if one string
else if (argc == 2)
{
    //all the remaining program code
}

While this is technically correct, the else if is totally unnecessary because argc must equal 2 at this point. Likewise, a simple else is also unnecessary because if argc !=2, then the program terminates and the code never gets this far.

WHY is it a bad practice? In the real world, code is frequently modified over it's useful lifetime. When it gets modified, it is likely done long after it was created, and the original author might not remember the finer points of a program, or it might be modified by someone that's never seen the code before. Surrounding the bulk of the code with an unnecessary if or else condition, and their curly braces {}, can and often is an opportunity for bugs to sneak into code when it's modified. Those curly braces are unnecessary and needlessly complicate the code. A modification could lead to someone adding more code after the closing brace when it should be inside, or inside when it shoould follow. Simply put, it's always best to keep code as simple as possible.

In line with that, this line does the same - it surrounds all the remaining code in an if block needlessly.

        if (i == strlen(argv[1]))

When testing for something that must be true or false, best practice is to make a small test that terminates the program, rather than a test that must surround the remaining code.

This tests really checks to see if all the chars in the key have been checked. (It could be argued that this test isn't necessary since the prior for loop MUST check them all or the program will terminate with an error. But it makes for more bulletproof.) So, how could you make a check for this in a few lines and not surround the remaining code?

        if (i != strlen(argv[1]))
        {
            printf("error message \m")
            return 1;
        }

This would end the program if the condition isn't met, and compartmentalizes the check in a few lines of code instead of spreading it across all the code.

In line with keeping code as simple as possible, there is opportunity to simplify in the code above, starting with removal of needless variables and associated calculations and/or assignments.

The bool valid_key appears 3 times, is set to true each time, and is not used for anything. It could be removed completely and change nothing.

These are the kinds of things to avoid when writing code. It's also a good practice at this stage to go back and look at your code (preferably after a good night's sleep) with a fresh eye after a break, and look for unnecessary or overly complex code and see if it can be simplified.

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

1
  • Thank you so much!! It works :) May 13 '20 at 11:19
1

The 'for' loop used to validate the key, should finish before ask the user for the input string. Then you start the second loop to process the input and return the encoded characters.

7
  • Thank you but I just did that and it is still saying the same thing.. :( May 12 '20 at 16:35
  • Could you post the code so I can take a look?
    – Tritum
    May 12 '20 at 16:55
  • Sorry how do I post the code further than I have already done? I'm new here May 12 '20 at 17:16
  • I think you can edit your original cuestion. You could add the new code at the end
    – Tritum
    May 12 '20 at 17:19
  • okay I added the new code at the bottom May 12 '20 at 19:01

You must log in to answer this question.

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