Re: encryption problem

dogatemycomputer@gmail.com wrote:


  Your code is really horrific indeed.
  This has little to do with C++ specifically and much more to do
with (procedural) programming in general, and I'm not completely sure
how on-topic this is in this group, but anyways.


  The header for C string manipulation functions is <cstring>,
not <string.h>.


  Just a piece of advice: Even though it's tempting to write that
"using" line, don't. It will only teach you bad habits in the long
run. Those namespace prefixes are not so tedious.


  What if you want to also support lowercase characters?

  The <cctype> standard library has many handy functions for
checking and converting characters. For example, isalpha(),
isupper(), islower(), tolower(), toupper() and so on. You should
get acquainted with those.

  Anyways, even though technically not supported by the C++ standard
(as far as I know), in practice 100% of systems (at least those which
you care) use the ASCII codes for the first 127 characters in the
character set (even if the input is UTF-8 encoded).
  Add to this that in C++ for example 'A' means the ascii value of
that character, and suddenly you can check if a certain character
is, for example, between A and Z by a simple (c >= 'A' && c <= 'Z').

  Another feature in C++ is that characters are simply integer values
and thus support integer arithmetic. This allows an ample set of nice
conversions to be done with simple math. For example, if you want to
map 'A' to 'Z', 'B' to 'Y' and so on, you can do it with a simple
'Z'-(c-'A'). The (c-'A') part maps the character from the range
'A'-'Z' to the range 0-25, and this value is then substracted from
the ascii value 'Z'. The result is that 'A'...'Z' gets mapped to
'Z'...'A'.

  If you want to make a rotation instead of an inversion, you can
simply perform an addition and modulo, but I'll leave that for now.

  Another nice trick is that since character values are just integers,
character values can be used to index arrays. Thus if you have checked
that c is indeed inside the range 'A'...'Z', then you can use (c-'A')
(which, as mentioned above, results in values between 0 and 25) to
index an array of 26 elements. Thus if c equals 'A' the first element
will be indexed, 'B' would index the second one and so on.


  Have you ever heard of the term "buffer overflow"? That kind of
code you have written above is exactly one of the causes. Don't do it.

  You should *always* think: "What happens if the user enters more
than 80 characters?" And no, the solution is not to increase the
size of that array to some thousands of characters.

  In C++ there's no need to use fixed-sized arrays for text because
there's this useful class called std::string. It will dynamically
grow itself as necessary and thus buffer overflows can be avoided.
Another plus side is that std::string is way easier to use than
char arrays and C string functions. It's usually a lot faster, too.

  Here is the critical point. What happens if the user enters more
than 80 characters?
  You don't want to do that. You want to do this:

std::string plainText;
std::cout << "text: ";
std::getline(std::cin, plainText);

  Now it doesn't matter how much text the user writes. It won't cause
a buffer overflow.


  First advantage of using std::string: That line is obsolete.

  plainText.length() will give the length of the string when you need
it. And way faster than strlen().

  Have you given any thought about how inefficient that code is?

  For each character in the input you loop through the "decrypted"
array, each time recalculating its size with strlen. Even if the
compiler is so smart as to be able to optimize the strlen outside
the loops you are still making 26 loops for each character in the
input. This is completely unnecessary and needlessly complicated.

  You don't need to traverse any "decrypted" array. By doing so
you are actually calculating (plain_text[i]-'A'), which accomplishes
the same thing with one line and way faster.

  Consider this:

std::string plainText;
std::cout << "text: ";
std::getline(std::cin, plainText);
std::string converted;

for(unsigned i = 0; i < plainText.length(); ++i)
{
    char c = plainText[i];
    if(c >= 'A' && c <= 'Z')
        converted += encrypted[c-'A'];
    else
        converted += plainText[i];
}

std::cout << converted << "\n";

  In fact, if you are simply making the conversion and don't need the
original input text for anything else, you could as well modify the
original string instead of creating a new one, saving a few lines:

std::string plainText;
std::cout << "text: ";
std::getline(std::cin, plainText);

for(unsigned i = 0; i < plainText.length(); ++i)
{
    char c = plainText[i];
    if(c >= 'A' && c <= 'Z')
        plainText[i] = encrypted[c-'A'];
}

std::cout << plainText << "\n";

  Of course that allows the "encrypted" array to contain *anything*.
If you are simply making a character inversion, you can do it without
that array:

std::string plainText;
std::cout << "text: ";
std::getline(std::cin, plainText);

for(unsigned i = 0; i < plainText.length(); ++i)
{
    char c = plainText[i];
    if(c >= 'A' && c <= 'Z')
        plainText[i] = 'Z' - (c-'A');
}

std::cout << plainText << "\n";