Undefined identifier/ undeclared

Question

I recently started C++ and I wanted to create a simple ceaser cipher function that could be called on, however because I copied the coding structure form my python program I seem to be getting 4 errors and 2 warnings. The mode bit is a bool where true is to encrypt and false is to decrypt (it worked on python so hey) :).

First one on the line of me creating the function where "int" is, its saying "identifier "in" undefined"

Second one in the same line saying "expected a ')'"

Third one is after the 3 if statements, saying "identifier "CharPos" undefined" even though it is defined

And Forth on the same line saying "'CharPos': undeclared identifier"

#include <iostream>
#include <fstream>
#include <string>

std::string Encryption(std::string Password, int Key, bool Mode) {
    std::string Alphabet = "abcdefghijklmnopqrstuvwxyz0123456789";
    std::string EncryptPass = "";
    if (Key > 36) {
        Key = Key % 36;
    }
    for (int X = 0; X < Password.length(); X++) {
        if (Password.at(X) == ' ') {
            EncryptPass = EncryptPass + " ";
        }
        else {
            for (int Y = 0; Y < 36; Y++) {
                if (Password.at(X) == Alphabet.at(Y)) {
                    if (Mode == true) {
                        int CharPos = Y + Key;
                        if (CharPos > 35) {
                            CharPos = CharPos - 36;
                        }
                    }
                    if (Mode == false) {
                        int CharPos = Y - Key;
                        if (CharPos < 0) {
                            CharPos = CharPos + 36;
                        }
                    }
                    if (Mode != true and Mode != false) {
                        int CharPos = 0;
                    }
                    char CharPos2 = CharPos;
                    char EncryptChar = Alphabet.at(CharPos2);
                    EncryptPass = EncryptPass + EncryptChar;
                }
            }
        }
    }
    return EncryptPass;
}

Any help would be appreciated

Put a comment next to each line that has an error, instead of describing the location of the error. Also, paste the entire error message for each error. — cigien, Jun 17 '20 at 14:48
`int CharPos = 0;` is pointless. Remember the scope goes away when you hit the first } — drescherjm, Jun 17 '20 at 14:49
`char CharPos2 = CharPos;` the compiler is correct `CharPos` is not defined at this line. You defined 3 `CharPos` variables inside of three different scopes {} that have eneded. — drescherjm, Jun 17 '20 at 14:51
And `CharPos` will always be 0 by the end of the loop, because `Mode != true or Mode != false` is always true. — Yksisarvinen, Jun 17 '20 at 14:56
Ah thanks Yksisarvinen, minor oversight in my behalf, edited the post to make sense now thanks. Yeah my alphabet doesn't 94 characters your right :) I copied the general code from my python script that uses the ascii symbols as well making it a total of 94 whoops — Evorage, Jun 17 '20 at 16:31

Kerek · Accepted Answer · 2020-06-17T22:06:11.063

As noted above, the major issue you have in your code is that CharPos is redefined in each if clause.

Every place you put int CharPos = ..., you create a new variable, and even though its name is similar for you, for the compiler, there are three unique variables of this name. In order to use the same variable in all of the scopes - the environment of variables, you should define it once in the first common scope of all, that means, in the for loop before of the if - else clauses.

Also, as noted above Mode != true || Mode != false is equivalent to true!

I rewrote your code to be safe, achieve the wanted encryption, and more readable (IMO).


std::string encrypt(const std::string& word, std::size_t shift_amount, bool should_shift_up)
{
    static const std::string alphabet = "abcdefghijklmnopqrstuvwxyz0123456789";

    assert (shift_amount <= alphabet.size()); // makes no sense having shift > alphabet size!

    std::string encrypted_word(word.size(), '\0');

    for (std::size_t idx = 0; idx < word.size(); ++idx)
    {
        char original_char = word[idx];
        std::size_t pos_in_alphabet = alphabet.find(original_char);

        std::size_t shifted_char = 'a';
        if (should_shift_up)
        {
            shifted_char = (pos_in_alphabet + shift_amount) % alphabet.size();
        }
        else
        {
            shifted_char = (pos_in_alphabet > shift_amount)  
                         ? pos_in_alphabet - shift_amount
                         : alphabet.size() - shift_amount + pos_in_alphabet;
        }

        encrypted_word[idx] = alphabet[shifted_char];
    }

    return encrypted_word;
}

if I were to define CharPos after the for loop using Y but before the if - else statements would it create a unique variable every time it looped through, or should I define it at the very start as an integer and just change it during the process. It loops though every single letter of the alphabet for every single letter in the password — Evorage, Jun 17 '20 at 16:35
@Evorage if you initialize it on definition, it will re-initialize it each run of the loop, which seems fine in this case. I don't see an advantage to define it outside of the loop, since it doesn't need to be known in any other place! — Kerek, Jun 17 '20 at 16:37
thanks again it all seems to be working well when I did a test but no matter what key I used, it would return the last letter of the Ecrypted password as 0 - std::string Password = Encryption("Hello", 1, true); //test — Evorage, Jun 17 '20 at 16:46
change that to being last letter as 1 (I don't know if I can edit comments) — Evorage, Jun 17 '20 at 16:54
@Evorage There was indeed a bug, look at the code now, it works as intended! (Also, you can edit your comments by pressing the blue `Edit` text next to the comment (but you can't edit it if I have commented on it, only delete!). — Kerek, Jun 17 '20 at 22:06

ΦXocę 웃 Пepeúpa ツ · Answer 2 · 2020-06-17T16:31:35.427

you have some issues with the scope of some variables, like that here: char CharPos2 = CharPos; charPos is not in scope anymore so is actually an invalid assignment.

So instead of defining a new CharPost in every if else, declare it before and re assign it in every if check, like:

if (Password.at(X) == Alphabet.at(Y)) {
    int CharPos = 0;
    if (Mode == true) {
        CharPos = Y + Key;
        if (CharPos > 93) {
            CharPos = CharPos - 94;
        }
    }
    if (Mode == false) {
        CharPos = Y - Key;
        if (CharPos < 0) {
            CharPos = CharPos + 94;
        }
    }
    if (Mode != true or Mode != false) {
        CharPos = 0;
    }
    char CharPos2 = CharPos;
    char EncryptChar = Alphabet.at(CharPos2);
    EncryptPass = EncryptPass + EncryptChar;
}

but aside to that, your code is broken... so only transpiling is not working here...

look here:

std::string Alphabet = "abcdefghijklmnopqrstuvwxyz0123456789";
...
for (int Y = 0; Y < 94; Y++) {
    if (Password.at(X) == Alphabet.at(Y)) {

Alphabet at(Y) will explode for values higher than 36...

Weird name (Sorry) thanks I've edited my original post to the actual value of 36 (94 came from my python script that used symbols in the alphabet variable to give it a length of 94) — Evorage, Jun 17 '20 at 16:44
offtopic: you can ref/reply using the @ and letting stackoverflow suggest the name of the person... — ΦXocę 웃 Пepeúpa ツ, Jun 17 '20 at 16:47
ive never used stack overflow before really, ontopic - I did a couple of tests using - std::string Password = Encryption("Hello", 1, true); and it always returned the last value of the encrypted password as 1, any ideas? — Evorage, Jun 17 '20 at 16:50

Undefined identifier/ undeclared

2 Answers2