pset2 Vigenere problem, after last letter, it does not encrypt

Question

I've written this code. For now it wonly works with small caps letter, I will add Capital letters once I get it working. The main idea is that J will start at 0 and follow up one step so it can be used in the calculation for the: printf("%c", ((((p[J] + k[moduled]) % 26))+85 )); formula moduled is a variable that starts at 0 and it wraps around. Everything goes great until the last letter of the keyword. I've been at this problem over a week now and I can't seem to know the reason it does not cypher correctly at the last letter and from there on.

Here's the code and the result after compiling:

{
include <stdio.h>
include <cs50.h>    
include <string.h>
include <stdlib.h>
 include <ctype.h>


int main(int argc, string argv[])

{

    if (argv[1] != NULL)
        {
            string k = argv[1];
            int J = 0; // This variable will be the continious one, for K purposes
            for (int j = 0, n = strlen(k); j < n; j++)
            {
                char h = k[j];
                if(( h >= ' ' && h <= '@') || (h >= '[' && h <= '`') || (h >= '{' && h <= '~'))
                {
                printf("Please enter a keyword to start decryption!!\n");
                return 1;
                }
                else
                {
                }
            }
                    string p = GetString();
                    int largo = strlen(k);
                    for (int i = 0, N = strlen(p); i <= N; i++)
                    {
                        do
                        {
                        int moduled = (J % largo);
                            if (moduled == 0 && i == 0)// IF J != DE 0
                            {
                                if (p[i] >= 97 && p[i] <= 122)
                                {
                                printf("%c", ((((p[J] + k[moduled]) % 26))+85 ));
                                J = J +1 ;
                                }
                                else if (p[i] >= 65 && p[i] <= 90)
                                {

                                }
                                else if (p[i] == 32)
                                {
                                printf(" ");
                                }
                            }
                            else if ( moduled > 0)// 
                            {
                                if (p[i] >= 97 && p[i] <= 122)
                                {
                                printf("%c", ((((p[J] + k[moduled]) % 26))+85 ));
                                J = J +1 ;
                                }
                                else if (p[i] >= 65 && p[i] <= 90)
                                {

                                }
                                else if (p[i] == 32)
                                {
                                printf(" ");
                                }
                            }
                            else
                            {
                                if (p[i] >= 97 && p[i] <= 122)
                                {
                                printf("%c", ((((p[J] + k[moduled]) % 26))+85 ));
                                J = J +1 ;
                                }
                                else if (p[i] >= 65 && p[i] <= 90)
                                {

                                }
                                else if (p[i] == 32)
                                {
                                printf(" ");
                                }
                            }

                        }
                        while (i > N);
                    }
        }
    else
    printf("Please enter a KEYWORD to start decryption!!\n");
    return 1;
}
}

./vige2 bacon meet me negh an

Even without spaces ./vige2 bacon meetme negh`f

OK I-ve seen to have narrowed the problem: When I started doing some basic math to get to the desired letter using bacon and meet me at the park example,

m = 109 e = 101 e = 101 t = 116 m = 109 +

b = 98 a = 97 c = 99 o = 111 n = 110

207       198       200       227     219

If i % by 26 I get

 26        16        18        19      11

I Require the following Ascii values as the problem states

  110      101     103          104      122

So the difference is:

  85       85        85         85        12

So my formula for conversion is:

printf("%c", ((((p[J] + k[moduled]) % 26))+85 ));

WHY IS THE DIFFERENCE THE SAME ALWAYS EXCEPT FOR THE LAST LETTER, this is where I am getting the error!!

Please help it has been over a week working on this (including this last weekend)

Answer 1

I looked at it for a couple hours but never did figure out why it's failing after a space. I probably could track it down in time, but I think the time would be better spent redesigning the entire program. I'll try to explain why.

First, the meat of the program is contained in an if statement designed to check program usage. if (argv[1] != NULL) If the usage is wrong, the code to handle it is at the very end of the program. This is a poor design. If someone needs to work on this program at a later date, they will have to go hunting for the related code. They'll also have to understand much of the program to see this. It also introduces opportunities for bugs to get into the code.

Code should be compartmentalized to do a single task or a very few tasks whenever possible and not intermingled. Do small tasks in short blocks. Don't spread them across the entire program if it isn't necessary.

A far better approach is to simply make a test at the beginning of the program whether argc !=2. If not, print an error message and exit. If it is 2, then execution continues to more code that does not need to be inside of an if code block. In fact, the next block of self-contained code should check that the key is valid, which you do in a for loop, although there's more intermixing going on.

Next, you have a do/while loop nested inside a for loop. The do/while loop is totally unnecessary. It will always run exactly once. A do/while loop runs at least once, but the test condition will never be true, so it will never run twice. The do/while loop control statements can be removed with no impact other than making the code more efficient.

Now, you have three conditions with three blocks of code in your if/elseif/else statements. I have no idea what you're testing for when testing moduled. Having said that, all three blocks of code are identical, so there's no apparent purpose to doing those tests. It also seems like you're basing actions on the current position in the key string. This makes no sense to me.

The better approach is this. First, validate the key. Then, as a separate block of code, a for loop can loop through the plain text and process each character. If it's an alpha, encode it. If it's not an alpha, * whether it is a space, a punctuation mark, a number, or anything else*, echo it.

BTW, the only non-alpha that the code handled appears to have been a space. Also, isalpha(), islower(), and isupper() are your friends. ;-) Maybe toupper() and tolower() too.

So, maybe a rework is in order, along with a review of the week's class material, shorts, and the pset assignemnt videos.

This may have been brutal, but it's about the code and the learning process. I've always found that when I wrote flawed logic and fixed it with a redesign, I always learned and understood more.

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