C++ Caesar cipher

[sandorlev]

Hello!

I've started programming C++ a week ago, so I cannot solve my problems easily. I've started a small project for learning classes, it's about encryption and stuff. I don't really get pointers, although I've read some about them. My problem is that when I use the cipher, it replaces all characters in the string with an 'a' or an 'A'. Thus, the output of test.cpp is the following

LeVEnte A nEVeM
AaAAaaa A aAAaA
Press any key to continue . . .

Also, I'd like to learn how to write beatiful code, so if I do something the ugly way, please tell me!

//character.h
#pragma once

#ifndef _SLENCRYPT_CHARACTER_H_
#define _SLENCRYPT_CHARACTER_H_

const char SPACE = ' ';

const char LOWER_ALPHABET[26] = {
'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i',
'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r',
's', 't', 'u', 'v', 'w', 'x', 'y', 'z'
};

const char UPPER_ALPHABET[26] = {
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I',
'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R',
'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'
};

#endif

================================================

//cipher.h
#pragma once

#ifndef _SLENCRYPT_CIPHER_H_
#define _SLENCRYPT_CIPHER_H_

#include <string>
#include "character.h"

class SLEncryptCipher
{
	public:
		std::string text;
		void convertToLower()
		{
			int length = this->text.length();
			for (int index = 0; index < length; index += 1)
			{
				//This is for trying all characters in the alphabet
				for (int count = 0; count < 26; count += 1)
				{
					if (this->text[index] == UPPER_ALPHABET[count])
						this->text[index] = LOWER_ALPHABET[count];
				}
			}
		}
		void convertToUpper()
		{
			int length = this->text.length();
			for (int index = 0; index < length; index += 1)
			{
				//This is for trying all characters in the alphabet
				for (int count = 0; count < 26; count += 1)
				{
					if (this->text[index] == LOWER_ALPHABET[count])
						this->text[index] = UPPER_ALPHABET[count];
				}
			}
		}
		void removeSpaces()
		{
			std::string::size_type index = 0;
			bool spacesLeft = true;
			while (spacesLeft)
			{
				index = this->text.find(SPACE);
				if (index != std::string::npos)
					this->text.erase(index, 1);
				else
					spacesLeft = false;
			}
		}
};

//The Caesar cipher
class Caesar : SLEncryptCipher
{
	public:
		std::string text;
		Caesar(std::string text)
		{
			this->text = text;
		}
		void encipher(const int shift)
		{
			int length = this->text.length();
			for (int index = 0; index < length; index += 1)
			{
				for (int count = 0; count < 26; count += 1)
				{
					if (this->text[index] == LOWER_ALPHABET[count])
					{
						this->text[index] = LOWER_ALPHABET[(count + shift) % 26];
					}
					else if (this->text[index] == UPPER_ALPHABET[count])
					{
						this->text[index] = UPPER_ALPHABET[(count + shift) % 26];
					}
				}
			}
		}
};

#endif

================================================

//test.cpp

//test.cpp
#include <iostream>
#include "cipher.h"

using namespace std;

int main()
{
	Caesar test("LeVEnte A nEVeM");
	cout << test.text << endl;
	test.encipher(1);
	cout << test.text << endl;
	system("pause");
	return 0;
}

[vmanes]

When your encipher loop has found the letter and converted to the enciphered character, it keeps on checking that character. With a shift of one, if finds it again, changes it to the next letter, till it gets all the way to z/Z, and wraps it around to a/A at the end of the loop.

A couple ways you can fix this. One would be a boolean var in addition to the loop counter, something like:

void encipher(const int shift)
{
   int length = this->text.length();

   bool found = false;

   for (int index = 0; index < length; index += 1)
   {
      found = false;

      for (int count = 0; !found && count < 26; count += 1)
      {
         if (this->text[index] == LOWER_ALPHABET[count])
         {
            this->text[index] = LOWER_ALPHABET[(count + shift) % 26];
            found = true;
         }
         else if (this->text[index] == UPPER_ALPHABET[count])
         {
            this->text[index] = UPPER_ALPHABET[(count + shift) % 26];
            found = true;
         }
      }
   }
}

Another would be to use break statements when the conversion is done

for (int count = 0;  count < 26; count += 1)
      {
         if (this->text[index] == LOWER_ALPHABET[count])
         {
            this->text[index] = LOWER_ALPHABET[(count + shift) % 26];
            break;
         }
         else if (this->text[index] == UPPER_ALPHABET[count])
         {
            this->text[index] = UPPER_ALPHABET[(count + shift) % 26];
            break;
         }
      }

The break gets you out of the inner loop, but the outer loop will execute its next iteration.

[vmanes]

Oh, and another thought. What happens when a negative value is used as the shift?

[sandorlev]

Oh, and another thought. What happens when a negative value is used as the shift?

it should decipher it, but it doesn't... i don't know why.

[vmanes]

Let's say the shift is -5. What happens when you find a letter at index 2?

( 2 + (-5)) % 26 = ????

[sandorlev]

Let's say the shift is -5. What happens when you find a letter at index 2?

( 2 + (-5)) % 26 = ????

i've tried in in the python interpreter and it says

>>> (2 + (-5) % 26)
23

which should be right, but i don't get the good results when i try it in c++

[vmanes]

We're working in C++ here.
What does this do?

#include <iostream>
using namespace std;

int main( )
{
   cout << ( -3 % 26 );
   cout << endl;
   return 0;
}

[sandorlev]

We're working in C++ here.
What does this do?

#include <iostream>
using namespace std;

int main( )
{
   cout << ( -3 % 26 );
   cout << endl;
   return 0;
}

it returns -3, so the solution can be (26 - (shift % 26)), right?

[vmanes]

How about:
( 26 + (count + shift ) ) % 26