HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Vigenere cipher breaker

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
breakervigenerecipher

Problem

I am a relative newbie to C++. I recently finished a command line program which cracks Vigenere encoded messages from a file by testing each word in the dictionary.

I have never had a tutor/teacher for C++, so I wanted to make sure that I don't develop any bad habits.

Main.cpp:

#include 
#include 
#include 
#include 
#include "Alphanumeric.h" //turns characters into numerical equivalents
#include "Vigenere.h" //Vigenere Functions
#include 

#ifdef __APPLE__
    #define DICTPATH "/usr/share/dict/words"
#endif

int main(int argc, char* argv[])
{

std::cout  results;
    while (std::getline(dictionary, str))
    {
        float total = 0;
        float frequency[26] = {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0};

        std::cout  results;
        std::string keyword = argv[3];
        while (std::getline(dictionary, str))
        {
            std::cout << "key:";
            std::cout << str << std::endl;
            std::string result = Vigenere::DecryptVigenere(plain, str);

            if (result.find(keyword) != std::string::npos) {
                std::cout << "RESULT FOUND" << std::endl;
                results.push_back("Using " + str);
                results.push_back(result);
            }
        }
        std::cout << "*************RESULTS*************" << std::endl;
        for (std::string s : results)
        {
            std::cout << s << std::endl;
        }
        std::cout << "dictionary attack complete!!!" << std::endl;
    }
return 0;
}


Vigenere.h:

#ifndef VIGENERE_H_
#define VIGENERE_H_

namespace Vigenere
{

std::string DecryptVigenere(std::string ciphertext, std::string key);
std::string EncryptVigenere(std::string plaintext, std::string key);

}
#endif /* VIGENERE_H_ */


Vigenere.cpp:

```
#include
#include "Vigenere.h"
#include "Alphanumeric.h"

std::string Vigenere::DecryptVigenere(std::string ciphertext, std::string key)
{
std::string answer;
for (int i = 0; i < ciphertext.length(); i++)
{

Solution

Code design

It is perfectly fine if you're really new to C++, but I think that one of the worst thing you can do with C++ is to use it like C. If you don't use the object oriented paradigm, you are more likely to write C code with STL than actual C++.

If you need ideas for a second version of this exercise, you could try to design it in a way that take the code out of the main function to organise it in classes. This would also allow you to easily store and use your magic number "26" that you copy and pasted to several places (which is a very bad practise as it harms maintainability).

Error checking

It may be a good idea to check for command line parameters errors and give a feedback for it. You should check the length of argv[] to avoid any problem but you should also give a feedback to the user if his parameters do not meet what you expect.

Containers

You are using std::list to store the results but you seem to only use push_back with it. An std::vector would be a more appropriate container for your use.

Iterations

I love foreach loops too, but when you need to keep track of the index, it probably means that you should use a standard for loop. This is what you did when you iterated on the frequency array.

Context

StackExchange Code Review Q#148335, answer score: 3

Revisions (0)

No revisions yet.