codebreaking program

Uh, note that y = 0 assigns 0 to y. You probably want to compare instead, with y == 0.

Quote:

---

Originally Posted by boburob

Sorry im fairly new at c++ so alot of the stuff you said i didnt get:/

---

Not a problem. As both loops (outer and inner) loop from 0 to 25, use a for loop. Makes things more readable.

Quote:

---

Originally Posted by boburob

This code works out which letter in the text had the highest frequency, then it assigns e, the highest appearing letter in the alphabet, to the letters place in a new array decry, and continues doing this through this list

---

In addition to what lazerlight pointed out, please describe (in detail) what your variable "maxno" represents.

To shorten your code, define a string

Code:

---

const char chartable[] = "etaoinshrdlcumwfgypbvkjxqz";

---

chartable[y] will then be 'e' for y=0, 't' for y=1, ... 'z' for y=25.

You still don´t apply the advice you were given...

Please replace loops like

Code:

---

int x = 0;
while( x <= 25 )
{
  // do some stuff
  x++;
}

---

with

Code:

---

for( int x = 0; x <= 25; x++ )
{
  // do some stuff
}

---

What do you think does this line of code do:

Code:

---

if( y = 0 )
{
  // do something
}

---

PS:
Laserlight already answered this question. It´s good practice to place constants on the left side of a comparison, so the compiler yields an error message when you accidentally miss one = character:

Code:

---

if( a = 1 ) ... // ok, compiler won´t complain
if( 1 = a ) ... // compiler yields an error message because 1 is a constant

---

Ive made all the changes , hopefully its easier to understand now, both the arrays are working but its only outputting whatevers at the end of the decry array

Code:

---

#include <iostream>
#include <fstream>
//This section of the code includes the libaries needed to run the program
using namespace std;
// Im using standard namespaces through out the code


void codebreak(char code[513])
//This creates a function called codebreak, which uses the input code[]
{
        int x, lettotal, y, i, maxno, let[26], maxpos;
        //This creates all the integer variables used in my program, x, y and i are used for counting
        char decry[26], broke[513];
        //This creates the array used to hold the correct order of the alphabet
        const char chartable[] = "etaoinshrdlcumwfgypbvkjxqz";
        const char chartable2[] = "abcdefghijklmnopqrstuvwxyz";
        i = 0;
        maxno = 0;
        x = 0;
        lettotal = 0;
        y = 0;
        maxpos = 0;
        //This sets all the integers used to 0
        for (int x = 0; x <=25; x++){
                let[x] = 0;
        }
        //This assigns 0 to every value in the array let[25]
        for (int x = 0; x <= 512; x++){
                if (code[x] >= 'a' && code[x] <= 'z') {
                        let[code[x]-'a']++;
                }
        }
        //This section checks to see what charator is held in each element of the array code so I can work out the total letal frequncies
        for (int y = 0; y <= 25; y++){
                for (int i = 0; i <= 25; i++){
                        if (let[i] > maxno){
                                maxno = let[i];
                                maxpos = i;
                        }
                        //This code checks to see if the item in the array is currently larger than the maximum
                        //number(maxpos)
                }
                decry[maxpos] = chartable[y];
                let[maxpos] = 0;
                maxno = 0;
                maxpos = 0;
        }
        //This group of if statements is used to find out the correct sequence in the alphabet, by replacing the
        //letter with the highest frequency with e, etc.
        for (int x = 0; x <= 512; x++){
                for (int i = 0; i <=25; i++){
                        if (code[x] = chartable2[i]){
                                broke[x] = decry[i];
                        }
                }
        }
        //This loop is used to replace every letter in the array code[] with the correct replacement
        //according to frequency
        cout << "\nAfter code breaking:\n" << broke << endl;
        for (int i=0; i<26; ++i){
                cout << "" << i << ": " << decry[i] << '\n';
        }
}




int main()
//This defines the main() function
{
        char file[256];
        char code[513];
        int i;
        i = 0;
        //This defines the arrays and integers used in the main function
        ifstream inFile;
        //This open the stream inFile
        system("cls");
        //This clears the output screen
        cout << "                                Code Breaker!" << endl;
        cout << "Please enter the name and extension of the file you want to decode:" << endl;
        cin.getline ( file, 512, '\n');
        cout << "The name entered was : " << file << endl;
        //This code gets the user to enter the name of the file that they wish to decode
        inFile.open(file);
        //This opens the text file specified by the user
        if (!inFile) {
                cerr << "Unable to open file";
                exit(1); 
        }
        //The code checks to see if the file opened correctly, if not, the program will end
        while(inFile >> code[i] && i < 512){
                i = i + 1;
        }
        //This puts the data that was in the users file, into the array code
        cout << "This file contains the code : \n \n" << code << "\n" << endl;
        cout << "Now performing code breaking..." << endl;
        codebreak (code);
        inFile.close();
        return 0;
}

---

Thanks for all your help so far

Quote:

---

Originally Posted by boburob

Ive made all the changes , hopefully its easier to understand now, now the let array is not working although ive made no changes:S

---

Not yet ;) There are still two loops where you increment the loop variable inside the loop´s body.
Next check if you can eliminate some variables from your function, it looks like as if you don´t need them all.
You still don´t name your variables properly. It´s ok to name variables x, y and i when using them in loops, but you should give them meaningful names. The same applies to the char tables, why don´t you name them PlainCharTable and CipherCharTable (or similar)?

What´s the content of your let array?

Lol ok i think now all the changes are made, the let array is working, but its just outputting whatever was in the last position of the decry array

Code:

---

#include <iostream>
#include <fstream>
//This section of the code includes the libaries needed to run the program
using namespace std;
// Im using standard namespaces through out the code


void codebreak(char code[513])
//This creates a function called codebreak, which uses the input code[]
{
        int x = 0, i = 0, maxno = 0, let[26], maxpos = 0;
        //This creates all the integer variables used in my program, x and i are used for counting
        char decry[26], broke[513];
        //This creates the array used to hold the correct order of the alphabet
        const char ciphertable[] = "etaoinshrdlcumwfgypbvkjxqz";
        const char normaltable[] = "abcdefghijklmnopqrstuvwxyz";
        //This sets all the integers used to 0
        for (int x = 0; x <=25; x++){
                let[x] = 0;
        }
        //This assigns 0 to every value in the array let[25]
        for (int x = 0; x <= 512; x++){
                if (code[x] >= 'a' && code[x] <= 'z') {
                        let[code[x]-'a']++;
                }
        }
        //This section checks to see what charator is held in each element of the array code so I can work out the total letal frequncies
        for (int x = 0; x <= 25; x++){
                for (int i = 0; i <= 25; i++){
                        if (let[i] > maxno){
                                maxno = let[i];
                                maxpos = i;
                        }
                        //This code checks to see if the item in the array is currently larger than the maximum
                        //number(maxpos)
                }
                decry[maxpos] = ciphertable[x];
                let[maxpos] = 0;
                maxno = 0;
                maxpos = 0;
        }
        //This group of if statements is used to find out the correct sequence in the alphabet, by replacing the
        //letter with the highest frequency with e, etc.
        for (int x = 0; x <= 512; x++){
                for (int i = 0; i <=25; i++){
                        if (code[x] = normaltable[i]){
                                broke[x] = decry[i];
                        }
                }
        }
        //This loop is used to replace every letter in the array code[] with the correct replacement
        //according to frequency
        cout << "\nAfter code breaking:\n" << broke << endl;
        for (int i=0; i<26; ++i){
                cout << "" << i << ": " << decry[i] << '\n';
        }
}




int main()
//This defines the main() function
{
        char file[256];
        char code[513];
        int i = 0;
        //This defines the arrays and integers used in the main function
        ifstream inFile;
        //This open the stream inFile
        system("cls");
        //This clears the output screen
        cout << "                                Code Breaker!" << endl;
        cout << "Please enter the name and extension of the file you want to decode:" << endl;
        cin.getline ( file, 512, '\n');
        cout << "The name entered was : " << file << endl;
        //This code gets the user to enter the name of the file that they wish to decode
        inFile.open(file);
        //This opens the text file specified by the user
        if (!inFile) {
                cerr << "Unable to open file";
                exit(1); 
        }
        //The code checks to see if the file opened correctly, if not, the program will end
        while(inFile >> code[i] && i < 512){
                i = i + 1;
        }
        //This puts the data that was in the users file, into the array code
        cout << "This file contains the code : \n \n" << code << "\n" << endl;
        cout << "Now performing code breaking..." << endl;
        codebreak (code);
        inFile.close();
        return 0;
}

---

Quote:

---

Originally Posted by boburob

Lol ok i think now all the changes are made, the let array is working, but its just outputting whatever was in the last position of the decry array

---

Again: = and == are different things ;)

Some more comments: You should initialize maxno to -1, not to 0. Otherwise characters which do not appear in the code are not mapped at all (admitted: this won't affect the decrypted output). Likewise, set let[maxpos] to -1.

Another thing: Use

Code:

---

for (int i = 0; i < 26; ++i)

---

instead off

Code:

---

for (int i = 0; i <= 25; ++i)

---

It's does not make a difference, but the first syntax causes less confusion, as you will only have one number (26) in the code and never 25. Likewise with 512/513.

Thank you everyone for your help, it is now working:D