Short and Messy Polybius Square

I attempted to recreate the Polybius square, also called the Polybius checkerboard, which was used in Ancient Greece for cryptography. Since it is an uncommon cipher, it is nowhere on Code Review.

Although the program technically works, it ended up very messy.

#include
#include
#include
#include
#include
#include

std::string getChoice();
std::vector getCoords();

int main(){
std::string choice = getChoice();

if(choice == "e"){
std::cout alphaToInt = {
{'A', 11},
{'B', 12},
{'C', 13},
{'D', 14},
{'E', 15},
{'F', 21},
{'G', 22},
{'H', 23},
{'I', 24},
{'J', 24}, // same as I
{'K', 25},
{'L', 31},
{'M', 32},
{'N', 33},
{'O', 34},
{'P', 35},
{'Q', 41},
{'R', 42},
{'S', 43},
{'T', 44},
{'U', 45},
{'V', 51},
{'W', 52},
{'X', 53},
{'Y', 54},
{'Z', 55},
};

for(int i = 0; i coords = getCoords();
std::string output;

const std::string squareIJ[5][5] = {
{"A", "B", "C", "D", "E"},
{"F", "G", "H", "I", "K"},
{"L", "M", "N", "O", "P"},
{"Q", "R", "S", "T", "U"},
{"V", "W", "X", "Y", "Z"},
};

for(int i = 0; i getCoords(){
std::cout begin(streamCoords);
std::istream_iterator end;
std::vector coords(begin, end);
return coords;
}


The Polybius square is a simple way to assign characters numbers, and then "encrypt" and "decrypt" based off of those numbers. As shown in the unordered map and char array, the numbers correspond to the row and column on a 5x5 square:

The getChoice() function simply gets the e or d character to choose encryption or decryption.

The getCoords() function gets a string, and then tokenize

It's not that messy IMO, but there are a few things which you could have done better:

-
Always compile code with every warning turned on, and fix them. When compiled with -Wall, your code produces 2 same warnings:

warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

You should fix them.

-
Your choice variable really wants to be an enum instance:

enum class mode {
    encrypt, decrypt
};

-
You have some redundant code:

if(choice == "E") choice = "e";
// redundant if in the next line your lower casing every character in 'choice'

-
const correctness: alphaToInt can and should be const, as you're not going to modify it no matter what.

-
getCoords can be reduced considerably (it looks too bloated otherwise IMO):

std::vector getCoords(){
    std::cout { streamCoords }, std::istream_iterator{} };
}

-
squareIJ doesn't have to be a std::string[5][5], it can be a simple char[5][5]. Don't allocate memory if you don't have too (this might actually not allocate memory for the std::string, due to SSO actually).

-
Use auto to simply some variable definitions if you want to:

// std::vector coords = getCoords();
auto coords = getCoords();

// ...

-
Use ranged-based loop instead of index loop. For example,

for(int i = 0; i < coords.size(); ++i){
    std::string coord = coords[i];
    output.push_back(squareIJ[coord[0] - '1'][coord[1] - '1']);
}

becomes

for (const auto& coord : coords)
    output.push_back(squareIJ[coord[0] - '1'][coord[1] - '1']);

-
Include every necessary headers, you forgot ` for std::isspace`.

-
If you expect every character to be upper case, either (1) tell the user (not recommended) or (2) upper case the string yourself:

std::transform(input.begin(), input.end(), input.begin(), ::toupper);

StackExchange Code Review Q#153352, answer score: 4