Unexpected Printed Exclamation Mark - cs50 Substitution

Question

Completing cs50's Substitution project. The program is supposed to use a key inputed in the command-line to 'encode' a message (string). My program works fine but for some strange reason it prints an exclamation mark at the end of each cipher. I have no idea why it does this.

So for example, this is what my program prints:

$ ./substitution VCHPRZGJNTLSKFBDQWAXEUYMOI
plaintext:  hello, world
ciphertext: jrssb, ybwsp!

Instead of the correct

$ ./substitution VCHPRZGJNTLSKFBDQWAXEUYMOI
plaintext:  hello, world
ciphertext: jrssb, ybwsp
```
The program: 
```
#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <ctype.h>

int key_check(string key);
int char_repeat(string key);
int encryption(string text, string key);

// WHY DOES IT PRINT EXCLAMATION??

int main(int argc, string argv[])
{
    string key = argv[1];
    if (argc != 2)
    {
        printf("Ussage: ./substitution key\n");
        return 1;
    }
    // checking for n of command-line args
    else if (key_check(argv[1]) == 1)
    {
        return 1;
    }
    string text = get_string("plaintext:  ");
    encryption(text, key);
    return 0;

}


int key_check(string key)
{
    // check for valid key
    if (strlen(key) != 26)
    {
        printf("Key must contain 26 characters.\n");
        return 1;
    }

    // check for repeated characters in key
    else
    {
        for (int i = 0; i < strlen(key); i++)
        {
            for (int j = i + 1; j < strlen(key); j++)
            {
                if (toupper(key[i]) == toupper(key[j]))
                {
                    printf("Ussage: ./substitution key\n");
                    return 1;
                }
            }
        }
    }
    return 0;
}

int encryption(string s, string key)
{
    int position;
    printf("ciphertext: ");
    for (int i = 0; i < strlen(key); i++)
    {
        char c = s[i];
        if (c >= 65 && c <= 95)
        {
            position = c - 65;
            printf("%c", key[position]);
        }
        else if (c >= 97 && c <= 122)
        {
            position = c - 97;
            printf("%c", tolower(key[position]));
        }
        else
        {
            printf("%c", c);
        }
    }
    printf("\n");
    return 0;
}

Thank you in advance!

Answer 1

Using the index i in the range [0, strlen( key ) ) for the string s within the function encryption invokes undefined behavior

for (int i = 0; i < strlen(key); i++)
{
    char c = s[i];
    //...

It seems you mean

for ( size_t i = 0, n = strlen( s ); i < n; i++ )

Or that is better

for ( size_t i = 0; s[i] != '\0'; i++ )

Pay attention to that it is a bad idea to use magic numbers like 65 in the program. Instead for example the magic number 65 you could use the integer character constant 'A' that makes the code more clear and readable.

You have a bug using the magic numbers in this if statement

if (c >= 65 && c <= 95)

The difference between 95 - 65 + 1 is not equal to 26 that is the length of the string key.

Also the return type int of the function

int encryption(string text, string key);

does not make a sense.

It would be better to declare the function like

string encryption(string text, string key);

and to return the modified string text.