Effecient Code

I recently wrote a program to do some cheesy decryption of my homework that my US history teacher gave us based on the Zimmerman Note. I thought up of making a two-dimensional array since that represented the key that he gave us and then assign each one a unique letter. Though it works, I'm looking for ways to code more efficiently, and with better design. Any tips on how to improve my code?

Code:

#include <iostream>
#include <string>

using namespace std;

int getGridNumber(char gridLetter)
{
    switch (gridLetter)
    {
        case 'a':
            return 0;
        case 'd':
            return 1;
        case 'f':
            return 2;
        case 'g':
            return 3;
        case 'v':
            return 4;
        case 'x':
            return 5;
        default:
            return 0;
    }
}

string decode(string argv)
{
    char grid[6][6];
    char char1;
    char char2;
    string news;

    grid[0][0] = 'B';
    grid[1][0] = 'I';
    grid[2][0] = '3';
    grid[3][0] = '7';
    grid[4][0] = 'M';
    grid[5][0] = 'U';
    grid[0][1] = '2';
    grid[1][1] = '9';
    grid[2][1] = 'D';
    grid[3][1] = 'H';
    grid[4][1] = 'O';
    grid[5][1] = 'V';
    grid[0][2] = 'E';
    grid[1][2] = 'N';
    grid[2][2] = '4';
    grid[3][2] = '8';
    grid[4][2] = 'P';
    grid[5][2] = 'W';
    grid[0][3] = '5';
    grid[1][3] = 'A';
    grid[2][3] = 'F';
    grid[3][3] = 'J';
    grid[4][3] = 'Q';
    grid[5][3] = 'X';
    grid[0][4] = 'R';
    grid[1][4] = '1';
    grid[2][4] = '6';
    grid[3][4] = '0';
    grid[4][4] = 'S';
    grid[5][4] = 'Y';
    grid[0][5] = 'L';
    grid[1][5] = 'C';
    grid[2][5] = 'G';
    grid[3][5] = 'K';
    grid[4][5] = 'T';
    grid[5][5] = 'Z';


    for ( int i = 0; i < argv.size(); i+=2)
    {
        news = argv.substr( i,2 );
        news.copy(&char1,1,0);
        news.copy(&char2,1,1);
        cout << grid[getGridNumber(char1)][getGridNumber(char2)];
    }
    return news;
}

int main ( int argc, const char* argv[])
{
    if (argc < 2)
    {
        cout << "Type -help because you fail. E.G. for retards: DecodeHomework.exe -help" << endl;
        return 0;
    }
    else if (argv[1] == string("-d"))
    {
        decode(string(argv[2]));
    }
    else
    {
        cout << "Well it seems that you attempted to type something in the parameter slot be it wrong or right. Type -help n00b." << endl;
    }
    return 0;
}

The following word:

Code:

fgafaaavxadgavxvadaddvdddvgavxvdxdvddfafdxgxgddgavfdvxvaafxgdadxvddxdavxxvaaavdavxdavvgddxavdgdxgxvxvdvfvvafdxavafvxdxvdfda
fxavvvfavafvvvvvadgxvaxafdggxfxafavvadgdfvfaxvdvxxfdavxgdvaafxgdadxvdvfavafvfdgavafvxvdaxafdgxdafafvaaadgvvvvxvvddfvvgdvdavvxdfv
dvxdadxafaaafavdfvvvxvdavfgfgxfdgvvgddadffxvxvddffddx

should give you:

Code:

FEBRUARY221917TOVONECKHARDTMEXICOCITYBRITISHCRACKTOPSECRETCODEUSPRESSMAYLEAKGERMANPLOTWITHMEXICOPREPARETOLEAVE
EMBASSYONSHORTNOTICEBERNSTORFFWASHINGTONDC

Simplify your code by simplifying the subject by using your mind.

A human and a computer are no different, you simply just don't realize you work like a program, analyzing everything.

So I'm not too familiar with this code but, you should try and add the most important points in bullet points. The most important decoding features.

Like, perhaps you need some mathematical relation of your grid code, to make it more efficient.

If you think about it, you should be able to simplify this ,because your grid is composed of A-Z 0-9, so you can simplify it by dividing it up using a loop and some mathematical formula that you derive by analyzing patterns and common letters.

Here's a hint on making it shorter:

Code:

    char grid[6][6] = {'B','I','3','7','M','U',
                       '2' ...
    };

You can read the description of the historic method here.

In the task the message is only half-way encrypted, essentially this - quite surely in the final message there is no one-to-one correspondence between pairs of letters in cipher-text and characters of the plaintext. (You could do a full encrypted-decrypter for extra challenge.)

Also, Zimmermann note was not encrypted with this cipher.

On top of iMalc's suggestion:

Code:

    const static char grid[6][6] = {'B','I','3','7','M','U',
                                                    '2' ...
    };

will allows the compiler to only set up ONE array, rather than initialize the array each time the function is called.

Isn't this:

Code:

        news.copy(&char1,1,0);
        news.copy(&char2,1,1);

the same as:

Code:

        char1 = news[0];
        char2 = news[1];

In which case, you could, in theory, get rid of char1 and char2 with the relevant news[].
You could take this a step further by just extracting the right positions out of the original input string one at a time, using the array indexing operator.

In getGridNumber

Code:

        default:
            return 0;

You may want to indicate that this is a failure (unless it really is allowed to be anything and then is treated as the same as 'a' - but I don't think that is what you meant?)

Note: Aside from initializing the array once [or not at all because the compiler produces it as a correct content already], I doubt that any of my suggestions will provide any useful improvement in performance with a reasonably modern compiler.

--
Mats

Code:

     const static char grid[6][6] = {'B','I','3','7','M','U',
                                                    '2' ...};

This only works for one array. Otherwise I would have used this.

You can read the description of the historic method here.

In the task the message is only half-way encrypted, essentially this - quite surely in the final message there is no one-to-one correspondence between pairs of letters in cipher-text and characters of the plaintext. (You could do a full encrypted-decrypter for extra challenge.)

Also, Zimmermann note was not encrypted with this cipher.

Thank you for the information. I actually didn't know about any of this...

There is no correspondence or pattern so it doesn't have a chance of having a for loop.

SOOOO...The next best thing I could do is make a window that allows user input of unique letters in the grid and then have a function that encrypts it as well. For now though since I only wanted a little bit of practice and for it to do my homework, I'll leave it at this for another time. Perhaps eventually if I could add in an INI and then have a simple FOR LOOP that adds it to it instead of adding it directly through the program.

Code:

 
#include <iostream>
#include <string>

using namespace std;

int getGridNumber(char gridLetter)
{
    switch (gridLetter)
    {
        case 'a':
            return 0;
        case 'd':
            return 1;
        case 'f':
            return 2;
        case 'g':
            return 3;
        case 'v':
            return 4;
        case 'x':
            return 5;
        default:
            return 0;
    }
}

/*char getGridLetter(int one, int two)
{


}
*/
string decode(string argv)
{
    char grid[6][6];
    string news;

    grid[0][0] = 'B';
    grid[1][0] = 'I';
    grid[2][0] = '3';
    grid[3][0] = '7';
    grid[4][0] = 'M';
    grid[5][0] = 'U';
    grid[0][1] = '2';
    grid[1][1] = '9';
    grid[2][1] = 'D';
    grid[3][1] = 'H';
    grid[4][1] = 'O';
    grid[5][1] = 'V';
    grid[0][2] = 'E';
    grid[1][2] = 'N';
    grid[2][2] = '4';
    grid[3][2] = '8';
    grid[4][2] = 'P';
    grid[5][2] = 'W';
    grid[0][3] = '5';
    grid[1][3] = 'A';
    grid[2][3] = 'F';
    grid[3][3] = 'J';
    grid[4][3] = 'Q';
    grid[5][3] = 'X';
    grid[0][4] = 'R';
    grid[1][4] = '1';
    grid[2][4] = '6';
    grid[3][4] = '0';
    grid[4][4] = 'S';
    grid[5][4] = 'Y';
    grid[0][5] = 'L';
    grid[1][5] = 'C';
    grid[2][5] = 'G';
    grid[3][5] = 'K';
    grid[4][5] = 'T';
    grid[5][5] = 'Z';


    for ( int i = 0; i < argv.size(); i+=2)
    {
        news = argv.substr( i,2 );
        cout << grid[getGridNumber(news[0])][getGridNumber(news[1])];
    }
    return news;
}

int main ( int argc, const char* argv[])
{
    if (argc < 2)
    {
        cout << "Type -help because you fail. E.G. for retards: DecodeHomework.exe -help" << endl;
        return 0;
    }
    else if (argv[1] == string("-d"))
    {
        decode(string(argv[2]));
    }
    else
    {
        cout << "Well it seems that you attempted to type something in the parameter slot be it wrong or right. Type -help n00b." << endl;
    }
    return 0;
}

> This only works for one array. Otherwise I would have used this.
You mean it needs more { } for your compiler, to reflect the actual structure of the arrays.


Does this work?

Code:

    const static char grid[6][6] = { { 'B','I','3','7','M','U' },
                                     { '2' ...
    };

Yes, I'm sorry I missed that iMalc had missed the extra braces.

Instead of:

Code:

    for ( int i = 0; i < argv.size(); i+=2)
    {
        news = argv.substr( i,2 );
        cout << grid[getGridNumber(news[0])][getGridNumber(news[1])];
    }

How about:

Code:

    for ( int i = 0; i < argv.size(); i+=2)
    {
        cout << grid[getGridNumber(argv[i+0])][getGridNumber(argv[i+1])];
    }

Eliminates the substr, which is going to "cost" a lot more than then indexing operator.

Code:

        cout << "Type -help because you fail. E.G. for retards: DecodeHomework.exe -help" << endl;

And if you follow those instructions, what happens?

--
Mats

Sup guys...

If you type -help it does nothing. This was a personal program for the time being I'm going to keep it that way.

Also the idea on getting rid of the substring was a great idea. Not sure why I didn't think of something like that...

As for the idea for changing the multidimensional array, I went ahead and placed it inside of an INI. Unfortunately, I can easily access the ini but ffs I can't seem to add strings together lol....Inside of getGrid(), I'm trying to add together two string literals. It calls them pointers and gives me the error that I can't add two pointers.

Code:

#include <iostream>
#include <string>
#include <fstream>
#include <windows.h>

using namespace std;

char getGrid()
{
    char grid[6][6];
    for ( int i = 0; i < 6; i++)
    {
        for ( int j = 0; j < 6; j++)
        {
            char* szResult = new char[255];
            memset(szResult, 0x00, 255);
            int d = 3;
            char szKey = "grid[" + d + "][" + j + "]";
            cout << szKey << " " << i << " " << j << endl;
            GetPrivateProfileString("Grid", "grid[1][1]", "", szResult, 255, ".\\default.ini");
            grid[i][j] = (int)szResult;
        }

    }
    return **grid;
}

int getGridNumber(char gridLetter)
{
    switch (gridLetter)
    {
        case 'a':
            return 0;
        case 'd':
            return 1;
        case 'f':
            return 2;
        case 'g':
            return 3;
        case 'v':
            return 4;
        case 'x':
            return 5;
        default:
            return 6;
    }
}

char getGridLetter(int x)    //Can't return two values of course. Quick fix is to assign it to a value through a pointer.
{
    switch(x)
    {
        case 0:
            return 'a';
        case 1:
            return 'd';
        case 2:
            return 'f';
        case 3:
            return 'g';
        case 4:
            return 'v';
        case 5:
            return 'x';
        default:
            return NULL;
    }
}

void encode(string argv)
{
    bool resetFor;
    char char1;
    char char2;
    char grid[6][6];

    cout << "woot";
    **grid = getGrid();
    for ( int a = 0; a < argv.size(); a++)
    {
        resetFor = false;
        for ( int i = 0; i < 6; i++)
        {
            for ( int j = 0; j < 6; j++)
            {
                if (argv[a] == grid[i][j])
                {
                    cout << getGridLetter(i) << getGridLetter(j);
                    resetFor = true;
                    break;
                }
            }
            if (resetFor == true)
            {
                break;
            }
        }
    }
}

void decode(string argv)
{
    char grid[6][6];
    **grid = getGrid();

    for ( int i = 0; i < argv.size(); i+=2)
    {
        cout << grid[getGridNumber(argv[i+0])][getGridNumber(argv[i+1])];
    }
}

int main ( int argc, const char* argv[])
{
    cout << szKey;

    if (argc < 2)
    {
        cout << "Type -help because you fail. E.G. for retards: DecodeHomework.exe -help" << endl;
        return 0;
    }
    if (argv[1] != "")
    {
        char arg;
        arg = argv[1][1];
        switch (arg)
        {
            case 'd':
            {
                decode(argv[2]);
                break;
            }
            case 'e':
            {
                encode(argv[2]);
                break;
            }
        }
    }
    else
    {
        cout << "Well it seems that you attempted to type something in the parameter slot be it wrong or right. Type -help n00b." << endl;
    }
    return 0;
}