0

I'm making a program for enciphering and deciphering text phrases that utilizes a 2D table. I have a single class that holds everything necessary for the cipher. However, I'm running into trouble when dealing with the table. I've got it constructed okay enough, but I'm having trouble encapsulating it in the class. I feel like it should be constructed automatically when an object is created, but right now I'm having to call it in through the main.

#include <iostream>
#include <string>
using namespace std;

class Cipher {
    public:
        Cipher();
        Cipher(string key);
        Cipher(string key, string message);

        void buildTable(char table[][26]);
        void newKey(string keyphrase);
        void inputMessage();
        string encipher(string message);
        string decipher(string message);
        string getPlainText() const;
        string getCipherText() const;

    private:
        string key;
        string plaintext;
        string ciphertext;
};

. . . .

void Cipher::buildTable(char table[][26]) {
    char alphabet[26] = {'a','b','c','d','e','f','g','h','i','j','k','l','m',
                        'n','o', 'p','q','r','s','t','u','v','w','x','y','z'};

    int alphaIndex = 0;

    for (int index1 = 0; index1 < 26; index1++) {
        for (int index2 = 0; index2 < 26; index2++) {   

            if ((index1 + index2) < 26) {
                alphaIndex = index1 + index2;
                table[index1][index2] = alphabet[alphaIndex];
            }
            else
                alphaIndex = 0;

            while (((index1 + index2) > 25) && index2 < 26) {
                table[index1][index2] = alphabet[alphaIndex];
                index2++;
                alphaIndex++;
            }           
        }               
    }
}

This table is key to the program functioning, and there is no reason for it to be changed. I tried including it as a private member, but ran into a lot of trouble. Should I be including this in the constructors, or what is the proper way to encapsulate this?

Victor
  • 57
  • 2
  • 8
  • I say make the implementation private using the PIMPL idiom. http://stackoverflow.com/questions/60570/why-should-the-pimpl-idiom-be-used Although I am confused on what problems you had when you made your table private. – drescherjm Jul 07 '14 at 16:07
  • As far as I understand, the table should be constant upon program invocation and shared across all instances of Cipher class, right? – user3159253 Jul 07 '14 at 16:07
  • 2
    Definitely should be a private member. What trouble are you running into? – T.C. Jul 07 '14 at 16:08
  • I'd suggest using a `std::array` for the cipher and making this a private member. – shuttle87 Jul 07 '14 at 16:13
  • @Victor, please choose an answer from the ones that have been provided. – aalimian Jul 07 '14 at 20:43

3 Answers3

0

The "char table[][26]" that you have, I recommend to make it a private member of your class. And when in the constructor you should initialize it. Your "buildTable" member function should not take the array as a parameter, but instead it should initialize your private 2 dimensional array. By looking at your code, I don't see any reason as to why you should have your table on the stack of "buildTable" function.

aalimian
  • 2,552
  • 1
  • 20
  • 42
  • Thanks, I will try this. – Victor Jul 07 '14 at 16:15
  • I think I was just staring at this for too long yesterday. I put the entire table construction in the constructor and it ran fine. Thanks for the help – Victor Jul 07 '14 at 16:21
  • well, I would not recommend to make the constructor so big. Instead, have your array initialization in a private "init" function and call the "init" function from your constructor – aalimian Jul 07 '14 at 16:22
  • Thanks I will do this! – Victor Jul 07 '14 at 20:05
0

This looks like a job for an Initialize On First Use function.

class Cipher {
    public:
        // ...
    private:
        using table_type = char[26][26];
        // Or for C++03 mode,
        // typedef char table_type[26][26];

        static void buildTable(table_type& table);
        static const table_type& getTable();
        // ...
}

const Cipher::table_type& Cipher::getTable() {
    static table_type the_table;
    if (the_table[0][0] == '\0')
        buildTable(the_table);
    return the_table;
}
aschepler
  • 65,919
  • 8
  • 93
  • 144
0

I prefer to leave the constructor parameters to feed-in the minimum class configuration values. So I would leave it as a private member. As for the values, I don't see why to re calculate them every time an instance is created, if they won't change, you just need to calculate them once and hard code them into the cpp file.

The *.h file:

class Cipher {
    ...
private:
    ...
    static const int table[26][26];
};

The *.cpp file:

...
const int Cipher::table[26][26] = { ... };
...
MJVC
  • 497
  • 4
  • 7