Problems with spaces in the char variable

[JoJoTag]

I want to encrypt spaces but a weird bug is taking place

-if you only input spaces, the default condition is used in the switch statement.

-if you type let's say

aa a

it will output

aaaa

like if space was an a.
if you type something like

a b

it will output

aba

.

I'm a little confused on what's causing that weird bug. Can anyone figure this out?

By the way, this is an extremely short version of my encryption program so that's the explanation for all these random includes and variables

Sry for my bad england

code:

#include <iostream>
#include <fstream>
#include <string>
#include <sstream>
#include <ctime>
#include <cstdlib>
#include <windows.h>

using namespace std;

int main()
{
	system("color A");

	int randNum;
	int msgCount;
	string msg; //normal msg
	string emsg;//encrypted msg
	char letter;
	char choice;

	//generate our random seed number based on time
	srand(time(0));

	//message input
	cout << "Decrypted:\n\n";
	getline(cin, msg);

	//setting up msgCount
	msgCount = msg.size();

	//encrypting the string
	for (int x = 0; x < msgCount; x++)
	{
		//convert string to char
		ss << msg;
		ss >> letter;

		//convert chars to encrypted text
		switch (letter)
		{
		case 'a'://a-z
			randNum = 1 + (rand() % 5);
			if (randNum == 1) { emsg += "a"; }
			if (randNum == 2) { emsg += "a"; }
			if (randNum == 3) { emsg += "a"; }
			if (randNum == 4) { emsg += "a"; }
			if (randNum == 5) { emsg += "a"; }
			break;

		case 'b'://a-z
			randNum = 1 + (rand() % 5);
			if (randNum == 1) { emsg += "b"; }
			if (randNum == 2) { emsg += "b"; }
			if (randNum == 3) { emsg += "b"; }
			if (randNum == 4) { emsg += "b"; }
			if (randNum == 5) { emsg += "b"; }
			break;

			//space

		case ' ':
			randNum = 1 + (rand() % 5);
			if (randNum == 1) { emsg += " space "; }
			if (randNum == 2) { emsg += " space "; }
			if (randNum == 3) { emsg += " space "; }
			if (randNum == 4) { emsg += " space "; }
			if (randNum == 5) { emsg += " space "; }
			break;

		default:
			system("cls");
			cerr << "Unknow key: " << letter << endl;
			system("pause");
			return 1;
		}
	}
	cout << "\n\n\nEncrypted:\n\n";
	cout << emsg << endl << endl;
	system("pause");

	return 0;
}

[Chervil]

I'm not sure of the purpose of the stringstream here:

    for (int x = 0; x < msgCount; x++)
    {
        //convert string to char
        ss << msg;
        ss >> letter;

(edit: I suppose the stringstream, though just adding an extra layer of complexity, could work if it was assigned the value before the loop begins, rather than repeatedly inside the body of the loop. But still I suggest, remove it completely).


You could just do

    for (int x = 0; x < msgCount; x++)
    {
        //convert string to char
		
        char letter = msg.at(x);

Or msg[x] instead of the at() function.

Or better, a range-based for loop:

    for (char letter : msg)
    {
        // convert chars to encrypted text
        switch (letter)
        {

Notice I declared letter within the loop - it's good to limit the scope of variables to just the place where they are used. it avoids clutter and confusion in the rest of the code.

[JoJoTag]

Right thanks, Chervil! I'll try the different methods you listed in your post and I'll let you know the results when I have time to code.

[JoJoTag]

Ay I fixed it all thanks to you Chervil! I optimized my code a little and this is how it looks like

#include <iostream>
#include <fstream>
#include <string>
#include <ctime>
#include <cstdlib>
#include <windows.h>

using namespace std;

int main()
{
	system("color A");

	string msg; //normal msg
	string emsg;//encrypted msg
	char choice;

	//message input
	cout << "Decrypted:\n\n";
	getline(cin, msg);

	//encrypting the string
	for (int x = 0; x < msg.size(); x++)
	{
                int randNum;

		//generate our random seed number based on time
		srand(time(0));

		//convert chars to encrypted text
		switch (msg[x])
		{
		case 'a':
			randNum = 1 + (rand() % 5);
			if (randNum == 1) { emsg += "a"; }
			if (randNum == 2) { emsg += "a"; }
			if (randNum == 3) { emsg += "a"; }
			if (randNum == 4) { emsg += "a"; }
			if (randNum == 5) { emsg += "a"; }
			break;

		case 'b':
			randNum = 1 + (rand() % 5);
			if (randNum == 1) { emsg += "b"; }
			if (randNum == 2) { emsg += "b"; }
			if (randNum == 3) { emsg += "b"; }
			if (randNum == 4) { emsg += "b"; }
			if (randNum == 5) { emsg += "b"; }
			break;

		case ' ':
			randNum = 1 + (rand() % 5);
			if (randNum == 1) { emsg += " space "; }
			if (randNum == 2) { emsg += " space "; }
			if (randNum == 3) { emsg += " space "; }
			if (randNum == 4) { emsg += " space "; }
			if (randNum == 5) { emsg += " space "; }
			break;

		default:
			system("cls");
			cerr << "Unknow key: " << msg[x] << endl;
			system("pause");
			return 1;
		}
	}
	cout << "\n\n\nEncrypted:\n\n";
	cout << emsg << endl << endl;
	system("pause");

	return 0;
}

Instead of using the char variable to store the letter we want to encrypt, I use msg[x]

[Chervil]

One comment now that the previous problem is fixed.

You should call srand() just once at the start of main().

Because you have it inside a loop, which will probably execute in much less time than 1 second, on each pass of the loop, srand() will be re-seeded with the same value (because the value of time(0) is unchanged). That means all the 'random' numbers will usually be identical. Once in a while the program may execute at the very moment that the time changes from one second to the next, but that still only gives two seed values, hence all the random numbers will be one of two values.

The rest of the code could probably be improved too, but I'm not sure what algorithm you are using for the encryption, so it's hard to advise properly.

[JoJoTag]

Right, I'll fix that. Thanks again dude!