1
\$\begingroup\$

It's my university task. Can I get some opinions on it?

#include <iostream>
#include <vector>


using namespace std;

string toBinary(short n) {
   string r;
   while(n != 0) {
      r = ( n % 2 == 0 ? "0" : "1" ) + r;
      n /= 2;
   }
   return r;
}

int has_even_parity(int c) {
   int k = 0;
   if (c != 0)
      k++;
if ((c & 0x2) >> 1 == 1)
    k++;
if ((c & 0x4) >> 2 == 1)
    k++;
if ((c & 0x8) >> 3 == 1)
    k++;
if ((c & 0x10) >> 4 == 1)
    k++;
if ((c & 0x20) >> 5 == 1)
    k++;
if ((c & 0x40) >> 6 == 1)
    k++;
if ((c & 0x80) >> 7 == 1)
    k++;
 return k % 2;
}

char decode(short int a) {
  char result;
  unsigned short int mask = 0b0111111110000000;
  return (char)((a & mask) >> 7);
}

int main() {
short int result[64];
string str;
vector<string> text_list;

for (int i = 0; i < 8; i++) {
    getline(cin, str);
    text_list.push_back(str);
}
cout << endl;
int k = 0;

for (int i = 0; i < text_list.size(); i++) {
    string l = "";
    for (int j = 0; j < text_list[i].size(); j++) {
        short int c_ = text_list[i][j];
        short int l_ = 0b0000000;
        c_ = l_ | c_;
        result[k] = (short) has_even_parity(c_);
        result[k] = result[k] << 8;
        result[k] = result[k] | c_;
        result[k] = result[k] << 1;
        c_ = (short) (has_even_parity(i) ^ has_even_parity(j));

        result[k] = result[k] | c_;
        result[k] = result[k] << 3;
        result[k] = (short) (result[k] | i);

        result[k] = result[k] << 3;
        result[k] = (short) (result[k] | j);

        l += decode(result[k]);
        k++;
    }
    cout << l << endl;
}

return 0;
}

I have input as text 8 letters x 8 lines, 2 bytes for one letter.

  1. In bit 0-2 number of line

  2. In bit 3-5 number of position in line

  3. In bit 6 parity bit of 1) and 2)

  4. In bit 7-14 ASCII code of letter

  5. In bit 15 parity bit of 4)

And after, decode letter.

\$\endgroup\$
  • \$\begingroup\$ has_even_parity is returns true for c==2, which is incorrect. \$\endgroup\$ – CodesInChaos Oct 26 '16 at 10:12
4
\$\begingroup\$

using namespace std; is a bad habit. It's not that tedious to write std:: a few times and you get less name collisions on things you define.


Your indentation is inconsistent. This makes it difficult to follow the control flow.


The even parity can be discovered with repeated xors:

int has_even_parity(int c) {
   c ^= c >> 4;
   c ^= c >> 2;
   c ^= c >> 1;
   return ! (c&1);
}

short int l_ = 0b0000000;
c_ = l_ | c_;

This doesn't do anything.


I suggest adding some underscores in the binary literals to group the digits:

unsigned short int mask = 0b0111_1111_1000_0000;

This makes it easier to tell at a glance that there are 7 zeros at the end and that you meant for them to be there. Though once you are more comfortable with hexadecimal then 0x7f80 is just as clear.

\$\endgroup\$
  • \$\begingroup\$ Actually I'd suggest using Hex or Octal rather than binary constants. \$\endgroup\$ – pacmaninbw Oct 25 '16 at 13:40
  • 2
    \$\begingroup\$ @pacmaninbw I prefer binary over octal (at least until they adopt a 0o syntax for octal literals) \$\endgroup\$ – ratchet freak Oct 25 '16 at 13:53
  • \$\begingroup\$ Yeah, I prefer Hex myself, shorter and clearer, and it does have the 0x infront. Too easy to make a typo with binary. \$\endgroup\$ – pacmaninbw Oct 25 '16 at 13:54
  • 2
    \$\begingroup\$ @pacmaninbw though there's less space for unnoticed typos with digit grouping using _. \$\endgroup\$ – ratchet freak Oct 25 '16 at 14:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.