Effecient Code

This is a discussion on Effecient Code within the C++ Programming forums, part of the General Programming Boards category; I recently wrote a program to do some cheesy decryption of my homework that my US history teacher gave us ...

  1. #1
    Registered User
    Join Date
    Jan 2008
    Posts
    28

    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
    Last edited by Salem; 11-09-2008 at 12:45 PM. Reason: Folded coded lines for readability

  2. #2
    Registered User
    Join Date
    Apr 2006
    Posts
    137
    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.
    ★ Inferno provides Programming Tutorials in a variety of languages. Join our Programming Forums. ★

  3. #3
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    Here's a hint on making it shorter:
    Code:
        char grid[6][6] = {'B','I','3','7','M','U',
                           '2' ...
        };
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  4. #4
    The larch
    Join Date
    May 2006
    Posts
    3,573
    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.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  5. #5
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  6. #6
    Registered User
    Join Date
    Jan 2008
    Posts
    28
    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;
    }
    Last edited by computerquip; 11-10-2008 at 08:22 PM.

  7. #7
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,830
    > 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' ...
        };
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  8. #8
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  9. #9
    Registered User
    Join Date
    Jan 2008
    Posts
    28
    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;
    }
    Last edited by computerquip; 11-11-2008 at 10:07 PM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Enforcing Machine Code Restrictions?
    By SMurf in forum Tech Board
    Replies: 21
    Last Post: 03-30-2009, 08:34 AM
  2. Obfuscated Code Contest: The Results
    By Stack Overflow in forum Contests Board
    Replies: 29
    Last Post: 02-18-2005, 05:39 PM
  3. Obfuscated Code Contest
    By Stack Overflow in forum Contests Board
    Replies: 51
    Last Post: 01-21-2005, 04:17 PM
  4. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 06:06 PM
  5. Replies: 0
    Last Post: 02-21-2002, 06:05 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21