Not replacing all letters.

Pages: 12
Ch1156 (1999)
I am making a program that replaces all letters with a character, I have been working on this for a few days and i cannot seem to get it to work. The problem is that it does not replace all characters. I cannot figure out why. line 65 - 74 is where the problem is.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
 
#include <iostream>
#include <string>
#include <fstream>
#include <vector>
#include <limits>

void Encrypt(std::string& fileName, std::string& storeText);
void Decrypt(std::string& fileName, std::string& storeText);
void LoadText(std::string& fileName, std::string& storeText);

int main()
{
    std::cout << "What would you like to do?" << std::endl;
    std::cout << "1) Encrypt Text" << std::endl;
    std::cout << "2) Decrypt Text" << std::endl;

    int choice = 0;

    std::cin >> choice;

    std::cout << "Ok, now enter the name of the file" << std::endl;
    std::string fileName, storeText;

    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    std::cin.clear();

    getline(std::cin, fileName);


    LoadText(fileName, storeText);


    switch(choice)
    {
        case 1:
            Encrypt(fileName, storeText);
            break;

        case 2:
            Decrypt(fileName, storeText);
            break;
    }

    return 0;
}

void Encrypt(std::string& fileName, std::string& storeText)
{
    std::string 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"};
    std::string key[26] = {":#:", "*:&", "$::", "+!-", "??!", "**+", "[*]", "&<.",
                                 "<#>", "($$", ":<>", "->*", "$#^", "*^&", "@#+", "+-*",
                                 "^-*", ">+?", "?#%", "<<>", "?>?", ":::", ":;:", ":&:",
                                 ";*&", "&&&"};


    std::ofstream outputEncryption;
    outputEncryption.open(fileName.c_str());

    std::string::size_type position = 0;

    std::cout << "\nEncrypting this text\n" << std::endl;

    int loop = 0;

    while((position = storeText.find(alphabet[loop], position)) != std::string::npos)
    {
        storeText.replace(position, alphabet[loop].length(), key[loop]);
        if(std::string::npos)
        {
            position += alphabet[loop].size();
            loop++;
            position = 0;
        }
    }

    std::cout << storeText << std::endl; //Debug

    outputEncryption << storeText << std::endl;
}

void Decrypt(std::string& fileName, std::string& storeText)
{

}

void LoadText(std::string& fileName, std::string& storeText)
{
    std::ifstream openFile;
    openFile.open(fileName.c_str());

    while(!openFile.eof()) storeText.push_back(openFile.get());
}
Last edited on
newbiee999 (112)
Your program isn't working for me.

It asks me to choose "Encrypt or Decrypt" and after I choose one it asks me to enter the file name. Then I enter something random and it doesn't do anything afterwards.

But anyways, going through your code, I think the error is in line 65.
If you want to check if position equals something, you should use "==" instead of "=".
while((position == storeText.find(alphabet[loop], position)) != std::string::npos)


"=" is the assignment operator which sets a variable equal to something.
"==" is the relational operator which checks if two or more variables are equal or not.

Let me know if still this doesn't solve your problem.
Ch1156 (1999)
you need to have a file for it to be able to get the text from. also that didnt work.
Ch1156 (1999)
This seems to almost work.

1
2
3
4
5
6
7
8
9
10
11
 
while((position = storeText.find(alphabet[loop], position)) != std::string::npos)
    {
        storeText.replace(position, alphabet[loop].length(), key[loop]);
        position += alphabet[loop].size();
        loop++;

        if(position >= alphabet[loop].length())
        {
            position = 0;
        }
    }



EDIT: never mind, it doesnt lol.
Last edited on
Ch1156 (1999)
ok i have been trying to get this to work since i posted and i cannot figure it out.
nadurraXII (60)
easily the problem pertains to value types.... lol.... i'm sure you knew this. your going to have to think of a better solution then your while loop. your switching character and int.... work with these seperatly..
nadurraXII (60)
you can char them.
Ch1156 (1999)
im not sure i understand
closed account (48T7M4Gy)
For a start you should write your program so that it doesn't require a file to be loaded. Just derive a test string so you to test your encrypt/decrypt functionality.

As part of the testing run through each of the arrays you have setup and see whether you have the substitution part working. Do that first. Run through the alphabet and see whether you get the right result.

Good testing and debugging is unit testing not running through ever increasing lines of dross.

Hope this helps :)
Ch1156 (1999)
ok so i changed it so it uses a string instead of a file;

"As part of the testing run through each of the arrays you have setup and see whether you have the substitution part working"

How so? like, Just output it?


"easily the problem pertains to value types.... lol.... i'm sure you knew this. your going to have to think of a better solution then your while loop. your switching character and int.... work with these seperatly.."

I dont understand, bot arrays are strings though? They were chars but they gave me errors at run time so I changed them. Or are you talking about size_type position?


So What im thinking i need to do is loop through the entire arrays while still at the first position in the string, then when it's done looping it will continue to the next position and so on, right? I'm thinking of a for loop.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
 
#include <iostream>
#include <string>
#include <fstream>
#include <vector>
#include <limits>

void Encrypt(std::string& fileName, std::string& storeText);
void Decrypt(std::string& fileName, std::string& storeText);
void LoadText(std::string& fileName, std::string& storeText);

int main()
{
    std::cout << "What would you like to do?" << std::endl;
    std::cout << "1) Encrypt Text" << std::endl;
    std::cout << "2) Decrypt Text" << std::endl;

    int choice = 0;

    std::cin >> choice;

    std::cout << "Ok, now enter the name of the file" << std::endl;
    std::string fileName, storeText;

    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    std::cin.clear();

    getline(std::cin, fileName);


    //LoadText(fileName, storeText);


    switch(choice)
    {
        case 1:
            Encrypt(fileName, storeText);
            break;

        case 2:
            Decrypt(fileName, storeText);
            break;
    }

    return 0;
}

void Encrypt(std::string& fileName, std::string& storeText)
{
    std::string 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"};
    std::string key[26] = {":#:", "*:&", "$::", "+!-", "??!", "**+", "[*]", "&<.",
                                 "<#>", "($$", ":<>", "->*", "$#^", "*^&", "@#+", "+-*",
                                 "^-*", ">+?", "?#%", "<<>", "?>?", ":::", ":;:", ":&:",
                                 ";*&", "&&&"};


    std::ofstream outputEncryption;
    outputEncryption.open(fileName.c_str());

    std::string::size_type position = 0;

    std::cout << "\nEncrypting this text\n" << std::endl;

    int loop = 0;

    std::string text = "This is a string of text that is a debug line for testing out the encrypt function.";

    while((position = storeText.find(alphabet[loop], position)) != std::string::npos)
    {
        text.replace(position, alphabet[loop].length(), key[loop]);
        position += alphabet[loop].size();
        loop++;
    }

    std::cout << text << std::endl; //Debug

    //outputEncryption << storeText << std::endl;
}

void Decrypt(std::string& fileName, std::string& storeText)
{

}

void LoadText(std::string& fileName, std::string& storeText)
{
    std::ifstream openFile;
    openFile.open(fileName.c_str());

    while(!openFile.eof()) storeText.push_back(openFile.get());
}
Last edited on
TheIdeasMan (6817)
Hi Chay,

With the encryption it's easier to just append the encoded characters to a new string, rather than replace them.

I would make use of the tolower() function, that way your input can have upper & lower case chars.

With the switch, provide an option to Quit, and provide a default: case to catch bad input.

For your alphabet variable, it is an array of 26 strings which each have 1 char. So either make it an array of 26 char, or std::string alphabet = "abcdefghijklmnopqrstuvwyz"
You can use the [] or at() function to access individual chars in the string.
I would have these variables defined in main() , because both Encryption & Decryption need them.

I am having a lot of trouble understanding what you are doing on lines 67 - 72. Your current definition of alphabet is causing problems. On line 67 position is always 0 because the length of each "string" is 1 , line 70 has no effect because it reset on line 67. You could discover this by using a debugger. And searching for letters in the alphabet that aren't in the string means the loop will end. By this I mean that if your string is "mytext", there is no point in searching for a, b, c etc. Just encode each char as you read them - that is m,y,t,e,x,t. Because of all this confusion, I recommend the pseudo code below.

The array of strings for the encrypted values is OK, because there are 3 chars in each. Although one needs to enforce the constant size of these, as this is essential for decryption. For example, if one of them had 4 chars, the encryption would be wrong and decryption would fail.

Btw, when using a brace initialised array variable, there is no need to specify the size of the array, the complier can count them for you.

I strongly suggest writing pseudo code as comments before you write code - even though it seems boring as hell, it will help a great deal in writing code.


1
2
3
4
5
6
 
// create variable to hold alphabet chars
// create variable to hold encrypted values 
// find the position of the char in the text to be encoded, in the alphabet a = 0, b =1, c =2
// use the position to look up the value of the encrypted string
// append the encrypted string to the output string 
// write the output string to the file


I do this when I am writing something complicated, or if there is a documented method - I copy & paste the instructions as comments.

The good thing about writing pseudo code is that one can start off very general, then go back and put in more detail.

Another way to do this problem is to use a std::map . For encryption the key is the char in the alphabet, the value is the encrypted string. Obviously the opposite for decryption. So you will need 2 map variables. If you are clever, you can use the values in the first one to create the second one, seen as the value in one is the key in the other and vice versa. Have a look in the reference material to see how to use these.

Finally, don't use 
while(!openFile.eof())
to process to EOF, Google to see why this is no good. IIRC Duoas has written an article about this, and / or has written replies about it.



closed account (48T7M4Gy)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
 
#include <iostream>
#include <string>

int main()
{
    char alphabet[26] = {0};
    
    // A quick way of preparing an alphabet of ASCII chars (ints)
    for (int i = 'a'; i <= 'z'; i++)
        alphabet[i] = i;
        
    std::string key[26] = {":#:", "*:&", "$::", "+!-", "??!", "**+", "[*]", "&<.",
        "<#>", "($$", ":<>", "->*", "$#^", "*^&", "@#+", "+-*",
        "^-*", ">+?", "?#%", "<<>", "?>?", ":::", ":;:", ":&:",
        ";*&", "&&&"};
    
    std::string line = "This is a string of text that is a debug line for testing out the encrypt function.";
    
    
    for( int i = 0; i < line.length(); i++)
    {
        if ( line[i] >= 'a' && line[i] <= 'z') // can modify this for spaces and upper case, numbers etc
            std::cout << key[line[i] - 'a'];
    }
    
    return 0;
}
Ch1156 (1999)
With the encryption it's easier to just append the encoded characters to a new string, rather than replace them.


What do you mean exactly?


I would make use of the tolower() function, that way your input can have upper & lower case chars.


But if I do that how will i know what letters are capitilised when i decrypt it? I mean, maybe i could store the position of all the uppercase letters in a file, then when its reading it i can do toupper.


For your alphabet variable, it is an array of 26 strings which each have 1 char. So either make it an array of 26 char, or std::string alphabet = "abcdefghijklmnopqrstuvwyz"
You can use the [] or at() function to access individual chars in the string.


Ah ok i'll try that.


I am having a lot of trouble understanding what you are doing on lines 67 - 72. Your current definition of alphabet is causing problems. On line 67 position is always 0 because the length of each "string" is 1 , line 70 has no effect because it reset on line 67. You could discover this by using a debugger. And searching for letters in the alphabet that aren't in the string means the loop will end. By this I mean that if your string is "mytext", there is no point in searching for a, b, c etc. Just encode each char as you read them - that is m,y,t,e,x,t. Because of all this confusion, I recommend the pseudo code below.


You understand it more than I do lol. I was just trying different things to see if i could get it to work.


Btw, when using a brace initialised array variable, there is no need to specify the size of the array, the complier can count them for you.


Ok i'll change that.


I strongly suggest writing pseudo code as comments before you write code - even though it seems boring as hell, it will help a great deal in writing code.


Yeah it's pretty boring but i'll try it. Should i just write plain text ro include some code in it?


Another way to do this problem is to use a std::map . For encryption the key is the char in the alphabet, the value is the encrypted string. Obviously the opposite for decryption. So you will need 2 map variables. If you are clever, you can use the values in the first one to create the second one, seen as the value in one is the key in the other and vice versa. Have a look in the reference material to see how to use these.

Finally, don't use
while(!openFile.eof())
to process to EOF, Google to see why this is no good. IIRC Duoas has written an article about this, and / or has written replies about it.


Ok i'll look into both of those. Thanks.


@kemort

Your code crashes for me when I run it.
closed account (48T7M4Gy)
Your code crashes for me when I run it.

It runs OK on my machine, which doesn't help u much. Try it online by clicking on the gearwheel in the top right hand corner. It runs OK there too. Aside from that you should be able to debug it pretty easily if you get the gist of what I did. Cheers :)
TheIdeasMan (6817)
@Chay

Now that kemort has posted some code, you are probably better off running with that.

But if I do that how will i know what letters are capitilised when i decrypt it? I mean, maybe i could store the position of all the uppercase letters in a file, then when its reading it i can do toupper.



But you didn't have upper case chars in your alphabet string. So alter kemort 's code, so that they are included. If you have both upper & lower, then there is no need for either of tolower or toupper. I was just trying to get you to think about what your input was.

Yeah it's pretty boring but i'll try it. Should i just write plain text ro include some code in it?


Just write a "recipe" for all the steps you need to do the problem - like I did in my earlier post. Start out general, then go back and add more detail, until you are comfortable in converting the comments to code.

@kemort

Your code crashes for me when I run it.


Were there any warnings when you compiled it? Hopefully you have the warnings set to a high level. On g++ I routinely use -Wall -Wextra -pedantic

I am not sure if there might be (I haven't compiled it ), just asking - if there are, then these give clues as to why it might crash, so warnings need to be looked at and taken care of. As kemort says, you can use the debugger to figure out why there are runtime problems.

TheIdeasMan (6817)
What do you mean exactly?


You were using the replace algorithm, I was saying make a new string and write the encode value into it.

I compiled kemort's code in the cpp.sh there were 3 warnings.
closed account (48T7M4Gy)
I was aware of the 3 warnings online vs no warnings on my machine, but if you look at them they aren't significant and can be ignored. If anything they show up a crappy compiler here rather than anything wrong with the program I wrote as crappy as that might be.

The beauty about char's being int's is you can use them in the for loop so easily whether you use them directly or follow up the change of case suggestion using tolower etc :)
Last edited on
TheIdeasMan (6817)
@kemort


I didn't mean to be critical of your code, just that it crashed for the OP - so the first stop might have been to see if there were any compiler warnings, and then we all said to use the debugger to see why.

However, I still think that warnings should be looked into. The code worked on your machine, but the warning might lead to something that caused the program on someone else's machine to crash.

I am also aware that you ripped up something quick for the OP, but from my POV (for my code) if I have warnings - I haven't finished.

Interesting, there were no warnings on your compiler - if those warnings didn't show up, then maybe your warning level isn't high enough?




Ch1156 wrote:
You understand it more than I do lol. I was just trying different things to see if i could get it to work.


You really need to try to get away from doing this - Computer Science isn't in the business of trial and error.

Instead we try to think logically about what is happening - write it down with pen & paper if unsure. Look at what your input is, what data structure are you storing it in, what each function you call does exactly.

Also look at what the implications of your method (algorithm) are. For example, you were trying to use the replace function. Replacing 1 char with 3 messes up (or at least makes more complex) the calculating of the position of the next char to be encoded. That is why I recommended writing the encoded data to a new string. kemort's code wrote to std::cout , but you can easily change it so it appends to the encoded string variable.

Have you managed to write your pseudo code for the decryption part of your program? Should be pretty easy :+)
closed account (48T7M4Gy)
Good on you IdeasMan,

As I said, the warnings given here on the cpp shell can be ignored. In fact they don't make sense, even worse they are wrong.

You might have tried it out yourself instead of telling me what my first steps should be.

Cheers :)
TheIdeasMan (6817)
kemort wrote:
You might have tried it out yourself instead of telling me what my first steps should be.


Sorry, I meant the first steps for the OP would be to check for warnings. It is my bad - I have had some trouble getting my exact meaning across lately, the sentence was poorly written. Although my earlier post (directed at the OP) was to look at compiler warnings.

I still maintain that warnings should be looked at, let's go through them, (I used C++14, Wall Wextra, pedantic):

1. 12:57: warning: trigraph ??! converted to | [-Wtrigraphs]

Very Important, the output is changed from ??! to | , meaning later decryption would fail. This type of thing would be rather hard to spot.

2. 20:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

It's good practice to return a value of the right type (i.e. no implicit casting). size() returns a type of std::size_t

3. 6:10: warning: variable 'alphabet' set but not used [-Wunused-but-set-variable]
So this variable is not necessary. I know you had it for the benefit of the OP, still these types of warnings are handy.

I don't see any messages that are wrong, or don't make sense and I don't think any of them should be ignored, especially the one for line 12. Warnings are there for a purpose, the compiler doesn't write them just for laughs.

The cpp.shell uses gcc 4.9.2 which is not a "crappy compiler" , I used clang 3.7 and came up with the same warnings. Which compiler are you using, & what warning level?

Pages: 12