Nested for loop not returning to parent in substitution.c

Question

I have a nested for loop of

for(int i = 0; i < strlen(key); i++)
    {
        for(int j = i + 1; j < strlen(key); i++)
        {
            if(toupper(key[i]) == toupper(key[j]))
            {
                printf("cipher must have all unique characters\n");
                return 1;
            }
        }
    }

I've checked in debug50 and it tells me that for(int j = i + 1; < strlen(key); i++) just goes back to itself rather than going back to the parent loop and adding 1 to i before going back to the j loop which means it's just telling me that the second letter is equal to the second letter. My full code is here:

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



int main(int argc, string argv[])
{
    if(argc != 2)
    {
        printf("please enter two arguments in command-line\n");
        return 1;
    }
    string key = argv[1];
    for(int i = 0; i < strlen(key); i++)
    {
        if (!isalpha(key[i]))
        {
            printf("please use only letters\n");
            return 1;
        }
        if (strlen(key) != 26)
        {
            printf("cipher has to be 26 characters\n");
            return 1;
        }
    }
    for (int i = 0; i < strlen(key); i++)
        {
            for (int j = i + 1; j < strlen(key); i++)
            {
                if(toupper(key[i]) == toupper(key[j]))
                {
                    printf("cipher must have all unique characters\n");
                    return 1;
                }
            }
        }

    string plaintext = get_string("plaintext: ");
    for(int i = 0; i <strlen(key); i++)
    {
        if(islower(key[i]))
        {
            key[i] = key[i] - 32;
        }
    }
    printf("cipher: ");
    for(int i = 0; i < strlen(plaintext); i++)
    {
        if(isupper(plaintext[i]))
        {
            int letter = plaintext[i] - 65;
            printf("%c", key[letter]);
        }
        else if(islower(plaintext[i]))
        {
            int letter = plaintext[i] - 97;
            printf("%c", key[letter]);
        }
        else (printf("%c", plaintext[i]));
    }
    printf("\n");
}

any ideas on what I can do?

Answer 1

wouldn't the inner loop work better if it incremented j instead of i? ;-)

Programming tip: Look at the for loop setup:

    for(int j = i + 1; j < strlen(key); i++)

It's better to set a var equal to strlen when used in a for loop for two reasons. First, when it's written like that, strlen is called on every pass of the loop, using up cpu cycles. Second, if the string "key" is altered in such a way that a char is changed to 0x00, that would be interpreted as the end of string marker and the loop would end prematurely.

The best practice is to use this form:

    for(int j = i + 1, keylen = strlen(key);  j < keylen; i++)

In this form, strlen is calculated exactly once, before the first pass, and can never change (as long as keylen remains unchanged.) It saves cpu cycles and prevents the string length from changing throughout the loop execution.

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