I completed the caesar assignment on cs50 and tested it on my terminal and it worked perfectly, but on check50 is kept failing some tests.

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

int getkey(string k);
string cipher(string s, int key);
int key;
int p;
int q;

int main(int argc, string argv[])
{
    // Allow 2 command line inputs
    if (argc == 2)
    {
        // Assign a local string to allow char scan
        string s = argv[1];

        // Check if all inputs are numbers
        for (int i = 0; s[i] != 0; i  )
        {
            if (s[i] < 48 || s[i] > 57)
            {
                printf("Usage: ./caesar key\n");
                return 1;
            }
        }
        // Get key from string
        int cipherkey = getkey(s);

        // Get user text
        string text = get_string("plaintext: ");

        // Calculate ciphertext and print
        string ciphertext = cipher(text, cipherkey);
        printf("ciphertext: %s\n", ciphertext);
    }
    else
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
}

// Change string to int. Turns out theres already a function for this called atoi()
int getkey(string k)
{
    key = 0;
    for(int i = 0, conv = 0, n = strlen(k); k[i] != 0; i  , n--)
    {
        // Calcute the placevalue
        p = pow(10, n-1);
        conv = k[i] - 48; // Convert to int
        key = key   (conv * p); // Sum up
    }
    return key % 26;
}

// Cipher text
string cipher (string s, int key)
{
    for(int i = 0; s[i] != 0; i  )
    {
       if(islower(s[i]))
       {
           s[i] = s[i]   key;
           while(s[i] > 122)
           {
               s[i] = (s[i] - 123)   97;
           }
       }
       else if(isupper(s[i]))
       {
           s[i] = s[i]   key;
           while(s[i] > 90)
           {
               s[i] = (s[i] - 91)   65;
           }
       }
    }
    return s;
}

with error message

:) caesar.c compiles.
:) encrypts "a" as "b" using 1 as key
:( encrypts "barfoo" as "yxocll" using 23 as key
    output not valid ASCII text
:) encrypts "BARFOO" as "EDUIRR" using 3 as key
:) encrypts "BaRFoo" as "FeVJss" using 4 as key
:) encrypts "barfoo" as "onesbb" using 65 as key
:( encrypts "world, say hello!" as "iadxp, emk tqxxa!" using 12 as key
    output not valid ASCII text
:) handles lack of argv[1]
:) handles non-numeric key
:) handles too many arguments

I wrote the code without knowing the "atoi" function so i implemented a function called getkey() to return key. when i returned key normally, it failed.

:( encrypts "barfoo" as "onesbb" using 65 as key
Output not a valid ASCII text

Until i returned key % 26;

I dont know why check50 isnt working although the program works well on my terminal. Please help.

Updated code:

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

string cipher(string s, int key);

int main(int argc, string argv[])
{
    // Allow 2 command line inputs
    if (argc == 2)
    {
        // Assign a local string to allow char scan
        string s = argv[1];

        // Check if all inputs are numbers
        for (int i = 0; s[i] != 0; i  )
        {
            if (s[i] < 48 || s[i] > 57)
            {
                printf("Usage: ./caesar key\n");
                return 1;
            }
        }
        // Get key from string
        int cipherkey = atoi(s);

        // Get user text
        string text = get_string("plaintext: ");

        // Calculate ciphertext and print
        string ciphertext = cipher(text, cipherkey);
        printf("ciphertext: %s\n", ciphertext);
    }
    else
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
}

// Cipher text
string cipher (string s, int key)
{
    for(int i = 0; s[i] != 0; i  )
    {
        if(islower(s[i]))
        {
            s[i] = (int) s[i]   key;
            while(s[i] > 'z')
            {
                s[i] = (s[i] - 123)   97;
            }
        }
        else if(isupper(s[i]))
        {
            s[i] = (int) s[i]   key;
            while(s[i] > 'Z')
            {
                s[i] = (s[i] - 91)   65;
            }
        }
    }
    return s;
}

CodePudding user response：

I've rewritten the function trying to avoid going to far making corrections and improvements. Now, the copy is actually unnecessary, but I hope it helps you understand that a "signed (8 bit) char" is not 'wide' enough to use in calculations that lead to overflow...

Please read the following and try to follow along.

string cipher( string s, int key ) {
    for( int i = 0; s[i] != '\0'; i   ) {
        if( !isalpha( s[i] ) )
            continue;

        int copy = (int)s[i]; // unnecessary casting, but...

        // transform ASCII value into 0-25 range
        if( islower( s[i] ) )
            copy = copy - 'a';
        else // must be uppercase
            copy = copy - 'A';

        copy = (copy   key) % 26;

        // transform enciphered MODULO back into ASCII char
        if( islower( s[i] ) )
            copy = copy   'a';
        else // must be uppercase
            copy = copy   'A';

        s[i] = copy;
    }

    return s;
}

Quiet night, so I thought I'd rewrite the rewrite just to see what it looked like...

string cipher( string s, int key ) {
    for( char *cp = s; *cp; cp   )
        if( isalpha( *cp ) ) {
            char c = "Aa"[ !!islower(*cp) ];
            *cp = (char)(((*cp - c   key) % 26)   c);
        }
    return s;
}