patterncppModerate
Encryption/decryption program
Viewed 0 times
encryptionprogramdecryption
Problem
Here is a little encryptor I wrote, I'm just asking for people to tell me what is good practice and what's not. Also, I mean the code, not the actual encryption (because it's just a dumb thing).
Please tell me what I could do to make it better or just nicer design! :)
#include "stdafx.h"
#include
#include
using namespace std;
int encrypter(int input);
int decrypter(int input);
bool whatToDoFunc();
int main()
{
try
{
bool keepGoing = true;
while (keepGoing)
{
bool success = whatToDoFunc(); char redo;
cout > redo;
if (success == true && (redo == 'y' || redo == 'Y'))
keepGoing == true;
else
keepGoing = false;
}
}
catch (runtime_error err)
{
cout > exitCon;
return -1;
}
return 0;
}
bool whatToDoFunc()
{
int input;
char whatToDo;
cout > whatToDo;
if (whatToDo == 'e' || whatToDo == 'E')
{
cout > input;
int inputEncrypted = encrypter(input);
cout > input;
int numDecrypted = decrypter(input);
cout << "Your number decrypted is " << numDecrypted << endl;
}
else
{
cerr << "Error" << endl;
throw runtime_error("Your input was invalid");
}
}
return true;
}
int encrypter(int input)
{
int inputEncrypted;
inputEncrypted = ((((input * 2) + 7) + 5) + 8);
if (inputEncrypted % 2 == 0)
++inputEncrypted;
else
inputEncrypted = inputEncrypted + (13 + 13) + (11 + 11);
return inputEncrypted;
}
int decrypter(int inputEncrypted)
{
int decryptedNum = 0;
if (inputEncrypted % 2 == 1)
--inputEncrypted;
else
{
(11 - 11) - (13 - 13) - inputEncrypted;
}
decryptedNum = ((((inputEncrypted - 8) - 5) - 7) / 2);
return decryptedNum;
}Please tell me what I could do to make it better or just nicer design! :)
Solution
Bug?
It looks like there is a bug here:
After simplification, the expression becomes:
... which does nothing, as the result is not assigned, the value of
I don't know if this is your real intention (didn't look at your code close enough). But the original code does absolutely nothing, it's reduced to a statement whose result is not stored anywhere.
Use boolean expressions directly
This can be simplified:
To this:
Notice how you don't need to write
Naming
A name like
It looks like there is a bug here:
else
{
(11 - 11) - (13 - 13) - inputEncrypted;
}After simplification, the expression becomes:
else
{
-inputEncrypted;
}... which does nothing, as the result is not assigned, the value of
inputEncrypted is unchanged. If you wanted to negate the value of the variable, you would have to write like this:else
{
inputEncrypted = -inputEncrypted;
}I don't know if this is your real intention (didn't look at your code close enough). But the original code does absolutely nothing, it's reduced to a statement whose result is not stored anywhere.
Use boolean expressions directly
This can be simplified:
if (success == true && (redo == 'y' || redo == 'Y'))
keepGoing == true;
else
keepGoing = false;To this:
keepGoing = success && (redo == 'y' || redo == 'Y');Notice how you don't need to write
x == true, you can use boolean expressions directly.Naming
A name like
whatToDoFunc is obviously bad. Good function names are extremely important to understand how a program works.Code Snippets
else
{
(11 - 11) - (13 - 13) - inputEncrypted;
}else
{
-inputEncrypted;
}else
{
inputEncrypted = -inputEncrypted;
}if (success == true && (redo == 'y' || redo == 'Y'))
keepGoing == true;
else
keepGoing = false;keepGoing = success && (redo == 'y' || redo == 'Y');Context
StackExchange Code Review Q#113091, answer score: 15
Revisions (0)
No revisions yet.