Still being stuck in Vigenere

Question

https://pastebin.com/fghZJ7iQ

I have modified vigenere for many times within these three days. Still being stuck. Please tell me which step did I do it wrong?

Answer 1

Consider using the markdown here instead of linking to a pastebin. E.g.

code

Dumping your entire code and linking to pastebin or somewhere else is very problematic for a few reasons. People don't want to risk navigating to a virus laiden site.

Second, pasting here the specific code you want looked at constrains you to break up your problem into smaller chunks.

Third, related to the second and @MARS comment, your code has tremendous complexity. Even thought it might work. It's very difficult to learn from and others will have a very hard time reading it.

I suggest if you are using more than 2 if statements like you are, break up the logic into smaller components. Software development is best leveraged when you have logic that can be called over and over again.

To help you start out. Make a list of things on paper you need your program to do. Then incrementally design those things and fit them into the program.

I'll help you with an example:

1. Input from command line a key
2. create a function that will downcase the key
3. create a function that will take the downcase key and the message to create a cyphertext char
4. find a method to output the whole message

Consider writing independent programs that can do all these thing that you will later incorporate into your final problem set. e.g.

1. downcase.c
2. encrypt.c
3. output.c
4. commandLineInput.c

It's OK to have many more files other than your problem set submission in your IDE. The submission only looks for the file with the correct naming convention(caesar.c, vigenere.c)

Fourth, pasting your whole code like you did doeesn't help with academic honesty. It is very easy for someone to cheat when this happens.

Fifth, @MARS had a very difficult time going back and forth when you paste your code and end the question in an extremely general way like you did.

Still being stuck. Please tell me which step did I do it wrong?

Because there is so much to code to look at and nothing specific is asked, it will be impossible to find the problem, tell you what the problem is, you understand what the problem is, you implement the solution, and make it work bug free.

Please don't delete this thread, but accept one of the answers. Others should see what to do when they ask too general of questions.

Please consider following my previous suggestions on breaking up your code, and when you have a small piece of code (no more than 5-10 lines) that you can ask a specific question about go for it!

Post it in the chat rooms (slack / discord) under the right heading. I'll hang around those chat rooms and be on the look out for your question! I know you have a great problem in your code somewhere that you, me and others can learn from, but it's just too hard atm.

Answer 2

It is necessary to understand the specifications perfectly before starting to code, according to your code we have an external for loop:

 for (int l = 0, m = strlen(k); l < m; l++)

for each element l of this loop, the innermost loop:

 for (int i = 0, j = 0, p = strlen(plaintext); i < p; i++, j++)

will go through all the elements, this means that we will always use the same letter as a key, then change it and we will use the same letter as a key. This is not what is requested, please read the specifications well. Also you have a variable ( j ) that you never use, what is its function ?. In vigenere change the letter with each letter of the text in clear, when we reach the final character of the keyword we start again with the first character, this suggests the use of the module with the index associated with the keyword, but we should use it in the second for loop, what I want to tell you is that you eliminate the else clause of the first for. Try to recompose your program and if you have difficulties again do not hesitate to ask again.

EDIT:

There is a coding style problem with your code, which we can consider (at least I personally), not recommended. We have code blocks deeply nested, with conditions and loops that affect virtually the entire code, this hinders the comprehension and readability of the program, in addition to causing undesirable interactions as is the case, I consider it necessary to improve the readability to resolve your doubts . First of all we will separate the tasks in independent blocks, instead of in nested blocks. We first have to check that argc is two, why use an IF condition that affects the whole code? For me it is much simpler to separate the tasks and that the block only affects what we want to do at this moment:

if (argc != 2)
    {
       printf("Usage: ./vigenere <key>.\n");
       return 1;
    } 

We have isolated a block of code that does a task, and it ends there when the check is done, we do not worry about it anymore. After we check that the key is alphabetic, and for this we use a for loop (without nesting other loops or pieces of code), we will leave the program if it is not without using else clauses, we simply go out, the program continues with the next block if the keyword is alphabetic:

for(int l = 0, m = strlen(k); l < m; l++)
    {
        if (!isalpha(k[l]))
            {
                printf ("Please enter alpha.\n");
                return 1;
    }

Then we continue with the encryption of the text in clear, in its own block, here is where the difficulty of this problem comes, the key can be shorter than the clear text with which it is necessary to reuse the key, this is where you must squeeze the brain and think, if you fight hard enough you solve it, as a clue I'll tell you that you need two variables to iterate on, one for the clear text and one for the key, make sure to increase the key index only when the clear text is alphabetic (maybe a little different from how we normally do it?), as the key is probably shorter than the plain text, the use of the modulo operator will be useful, think about how to use it.