Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have been working on a Playfair Cipher in C++ for a final Project in a Cryptography class. I was hoping for some feedback on the project so far. I recently got a job programming before I've even graduated and I feel like I've already learned so much.

I would just like some feedback on my code style, comments, formatting, and structure.

The code below is the PlayfairCipher class. The code is fairly simple.

#include "PlayfairCipher.h"
#include "Sanitizer.h"
#include <iostream>
#include <string>
using namespace std;


    PlayfairCipher :: PlayfairCipher(void)
    {
        playfairMatrix.resize(5);
        for (int i = 0; i < 5; ++i) {
            playfairMatrix[i].resize(5);
        }
    }


    void PlayfairCipher :: createKeyMatrix(string &key) 
    {
        string alphabet = "abcdefghiklmnopqrstuvwxyz";
        string sanitizedKey = Sanitizer :: sanitizeInputKey(key);
        for(unsigned int i = 0; i < sanitizedKey.size(); i++) {
            for(int j = 0; j < alphabet.size(); j++) {
                if(sanitizedKey[i] == alphabet[j]) {
                    alphabet.erase(j, 1);
                }
            }
        }
        sanitizedKey.append(alphabet);
        int index = 0;
        for(int row = 0; row < 5; row++) {
            for(int column = 0; column < 5; column++) {
                playfairMatrix[row][column] = sanitizedKey[index];
                index++;
            }
        }
    }


    void PlayfairCipher :: print()
    {
        for(int row = 0; row < 5; row++) {
            for(int column = 0; column < 5; column++) {
                cout << playfairMatrix[row][column] << " ";
            }
            cout << endl;
        }
    }


    Point * PlayfairCipher :: getPointOfLetter(char letter) 
    {
        for(int row = 0; row < 5; row++) {
            for(int column = 0; column < 5; column++) {
                if(playfairMatrix[row][column] == letter) {
                    return new Point(row, column);
                }
            }
        }
        return NULL;
    }


    char PlayfairCipher :: getLetterOfPoint(Point * a) 
    {
        return playfairMatrix[a->row][a->column];
    }


    /////////////////////////////////////////////////////////////////////////////////
    /// First, a key matrix is created using the supplied key.                    ///
    /// Then the plaintext is sanitized to remove invalid characters.             ///
    /// For each pair of characters in plainText, retrieve their coordinates from ///
    /// the key Matrix. The coordinates are then transformed. Finally, the newly  ///
    /// transformed coordinates are used to retrieve the corresponding letter.    /// 
    /////////////////////////////////////////////////////////////////////////////////
    string PlayfairCipher :: Encrypt(string plainText, string key) 
    {
        createKeyMatrix(key);
        Sanitizer :: sanitizePlainText(plainText);

        for(unsigned int i = 0; i < plainText.size() - 1; i+=2) {
            Point * a = getPointOfLetter(plainText[i]);
            Point * b = getPointOfLetter(plainText[i + 1]);

            encryptCoordinates(a, b);

            plainText[i]   = getLetterOfPoint(a);
            plainText[i+1] = getLetterOfPoint(b);
        }
        return plainText;
    }


    /////////////////////////////////////////////////////////////////////////////////
    /// First, a key matrix is created using the supplied key.                    ///
    /// Then the plaintext is sanitized to remove invalid characters.             ///
    /// For each pair of characters in plainText, retrieve their coordinates from ///
    /// the key Matrix. The coordinates are then transformed. Finally, the newly  ///
    /// transformed coordinates are used to retrieve the corresponding letter.    /// 
    /////////////////////////////////////////////////////////////////////////////////
    string PlayfairCipher :: Decrypt(string cipherText, string key)
    {
        createKeyMatrix(key);

        for(unsigned int i = 0; i < cipherText.size() - 1; i+=2) {
            Point * a = getPointOfLetter(cipherText[i]);
            Point * b = getPointOfLetter(cipherText[i + 1]);

            decryptCoordinates(a, b);

            cipherText[i]   = getLetterOfPoint(a);
            cipherText[i+1] = getLetterOfPoint(b);
        }
        return cipherText;
    }


    void PlayfairCipher :: encryptCoordinates(Point * a, Point * b) {
        if(a->column == b->column) {
            a->row = (a->row + 1) % 5;
            b->row = (b->row + 1) % 5;
        }
        else if(a->row == b->row) {
            a->column = (a->column + 1) % 5;
            b->column = (b->column + 1) % 5;
        }
        else {
            int temp = a->column;
            a->column = b->column;
            b->column = temp;
        }
    }


    void PlayfairCipher :: decryptCoordinates(Point * a, Point * b) {
        if(a->column == b->column) {
            a->row = Mod(a->row - 1, 5);
            b->row = Mod(b->row - 1, 5);
        }
        else if(a->row == b->row) {
            a->column = Mod(a->column - 1, 5);
            b->column = Mod(b->column - 1, 5);
        }
        else {
            int temp = a->column;
            a->column = b->column;
            b->column = temp;
        }
    }


    int PlayfairCipher :: Mod(int x, int m) 
    {
        int r = x % m;
        return r < 0 ? r+m : r;
    }


    PlayfairCipher :: ~PlayfairCipher(void)
    {

    }

I have added the header file code as well:

#include <string>
#include <vector>
#include "Point.h"
using namespace std;

class PlayfairCipher
{
public:
    PlayfairCipher(void);
    string Encrypt(string plainText, string key);
    string Decrypt(string cipherText, string key);
    void print();

private:
    vector<vector<char>> playfairMatrix;
    void createKeyMatrix(string &key);
    void encryptCoordinates(Point * a, Point * b);
    void decryptCoordinates(Point * a, Point * b);
    char getLetterOfPoint(Point * a);
    Point * getPointOfLetter(char letter);
    int Mod(int x, int m);
    virtual ~PlayfairCipher(void);
};
share|improve this question
    
Can you post the header file so we can see the interface. –  Loki Astari Nov 9 '12 at 19:46
    
So, what will happen if I encrypt the word "jail"? For that matter, what if I encrypt using the key "jail"? –  Nick ODell Nov 13 '12 at 2:05
    
I didn't post it because I was just looking for comments on this class, but I have a another class called Sanitizer. This class has a few static methods that clean up input- whether it is the key or the plaintext. One method will translate all j's to i's. This technique is the one I've seen most often due to j's low letter frequency in the English language. –  Singularity222 Nov 13 '12 at 2:35
add comment

1 Answer

up vote 2 down vote accepted

Code looks clean and readable. 3 quick remarks:

  1. Magic numbers and string: Move'm aside. What's with that 5 ? Is that a cosmic constant, or something your boss may change tomorrow? Make it #define or extern.
  2. Defend your code. Your Point* creation method is allowed to return NULL, but you don't null-check your input.
  3. Code duplication - encrypt/decrypt coordinates are almost the same. Consider combining.
share|improve this answer
1  
5x5 is the size of the playfair cipher. I don't think it will change unless the number of letters in the English alphabet changes. –  Nick ODell Nov 13 '12 at 2:01
    
That is true, but I should have used a #define, it just makes it look a bit better, I think. –  Singularity222 Nov 13 '12 at 2:36
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.