0
\$\begingroup\$

I created my first ciphering program, which uses a quadratic equation as the encrypting algorithm. My goal was to make the code as minimalistic and readable as possible- what do you guys think? I am beginner, so please tell if this code represents any bad practices.

#include <iostream>
#include <string>
#include <cstdlib>
#include <math.h>
#include <map>

using namespace std;

int main()
{
std::map <char,int> val;

val['A','a'] = 1;   val['H','h'] = 8;    val['O','o'] = 15;   val['V','v'] = 22;
val['B','b'] = 2;   val['I','i'] = 9;    val['P','p'] = 16;   val['W','w'] = 23;
val['C','c'] = 3;   val['J','j'] = 10;   val['Q','q'] = 17;   val['X','x'] = 24;
val['D','d'] = 4;   val['K','k'] = 11;   val['R','r'] = 18;   val['Y','y'] = 25;
val['E','e'] = 5;   val['L','l'] = 12;   val['S','s'] = 19;   val['Z','z'] = 26;
val['F','f'] = 6;   val['M','m'] = 13;   val['T','t'] = 20;
val['G','g'] = 7;   val['N','n'] = 14;   val['U','u'] = 21;


string alphabet("ABCDEFGHIJKLMNOPQRSTUVWXYZ");
string sentence;
int a,b,c,x,y;

cout << "Enter a sentence to encode:" << endl;
getline(cin,sentence);


cout << "Input each modulus of encoding function: ax^2 + bx + c" << endl;
cout << "a = "; cin >> a;
cout << "b = "; cin >> b;
cout << "c = "; cin >> c;

for (unsigned i = 0; i <= sentence.length(); i ++)
{
    x = (val[sentence[i]]);
    y = a * pow(x,2) + b * x + c;

    if ((y >= 0)&&(sentence[i] != ' '))
        sentence[i] = alphabet[y % alphabet.length()];

    if ((y < 0)&&(sentence[i] != ' '))
        sentence[i] = alphabet[-(y % alphabet.length())];
}

cout << sentence;
}
\$\endgroup\$
4
  • 1
    \$\begingroup\$ We can review your existing enciphering code, but your request for deciphering is off-topic. \$\endgroup\$ Commented Oct 6, 2017 at 21:10
  • \$\begingroup\$ I realised that, the title is fixed now. \$\endgroup\$ Commented Oct 6, 2017 at 21:15
  • \$\begingroup\$ You should include a deciphering routine to demonstrate that it works, though. \$\endgroup\$ Commented Oct 6, 2017 at 21:19
  • 2
    \$\begingroup\$ He can't - it's impossible for all cases... Currently writing an answer... \$\endgroup\$ Commented Oct 6, 2017 at 21:22

2 Answers 2

Reset to default
4
\$\begingroup\$

Implementation

  • using namespace std; is considered bad practice. Try to avoid it.

  • val['A','a'] = 1 might not do what you think it does. It only creates an entry for 'a', not for 'A' (because of the comma operator), so you would have to split up that assignment.

  • Also, all of the initialization of val could be done by in a loop, so there is less code duplication.

  • i <= sentence.length() in the loop condition should be i < sentence.length(), as you currently overshoot the end of sentence.

  • std::pow does floating point exponentiation - not really needed in this case (could be replaced by x*x)

  • You first calculate an encoded value for spaces, only to discard them. It might be better to move the check for spaces up.

  • When your algorithm tries to encode an uppercase letter (or any character that is no lowercase letter), x gets set to 0, as that is the default value for int returned by std::map::operator[] and those values are not listed in val. This means that all those characters get encoded as the same value.

  • While it's fine for a simple program as this, you should consider extracting the core encoding functionality in a small set of functions so they can be reused.

Algorithm

The encoding algorithm is fundamentally flawed (it cannot be reversed unambiguously in the general case).

For an example, I have plotted the values for a = 1, b = -2, c = -3: Example plot

As you can see, most y-values have either 0 or 2 corresponding x-values. The exceptions are (1, 4), (14, 9) and (26, 23) corresponding to ('a', 'E'), ('n', 'J') and ('z', 'X') - for this selection of a, b and c.

Why that? There are multiple reasons:

  • The pigeonhole principle: You try to map 27 input values (0 to 26) onto 26 possible output values (0 to 25). So at least one output value must correspond to at least 2 input values.

  • x^2 by itself isn't uniquely bidirectional. This can be seen easily: The graph is a parabola, so most y-values (except the vertex) correspond to either 0 or 2 x-values. This gets worse by taking the absolute and by applying the modulo.

There are some nearly unique bidirectional mappings when a is a multiple of 26 and b and 26 are coprime and c/b must be greater than 0 or lower than -26 - this is because how the modulo operator works and how you handle negative y-values. They would be unique if you reduced the number of possible input values to 26, e.g. by moving the values for 'a' to 'z' down to 0 to 25. However, those mappings are basically linear (the contribution of a*x*x gets "removed" by the modulo 26). For b = 1, this would be a cesar cipher.

\$\endgroup\$
3
\$\begingroup\$

First, have you worked out how to reverse this encryption function? Are you sure it’s reversible?

using namespace std; is a bad habit and best avoided.

val['A','a'] = 1; does not do what you seem to think it does. It’s ignoring the first value 'A', because you’re using the comma operator. Also, doing it this way is wordy and error-prone. You can do the same thing with a loop:

for (int i = 1; i <= 26; ++i) {
    char uc = ('A' - 1 + i);
    char lc = ('a' - 1 + i);
    val[uc] = val[lc] = i;
}

The loop below is incorrect, it will go one past the end of the string. <= should be <.

for (unsigned i = 0; i <= sentence.length(); i ++)

This looks wrong:

-(y % alphabet.length())

You probably want to just use y % alphabet.length() either way.

\$\endgroup\$
1
  • \$\begingroup\$ There are some subtle differences between -(y % alphabet.length()) and y % alphabet.length(). In C++, the modulo of integers is defined so that a % b + b * (a / b) == a. For negative values of a and positive values of b, this means that a%b is negative - so your suggestion actually changes the sign of the result! \$\endgroup\$ Commented Oct 6, 2017 at 21:52

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.