patterncppMinor
Vigenere cipher breaker
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:
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++)
{
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
Containers
You are using
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
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.