Help in making a Caesar Cipher maker

I am designing a Caesar Cipher maker.
It will take characters in an array using cin.getline() then compare each character of the text with its numerical values (a = 1, b = 2...) and then add the second shift in the characters (if shift = 1;a = 1 [+1 = 2 = b]).
Well, that was what it is supposed to do.
So here's the problem:
When I enter the text TEST and the shift for the code 2
I get
vugg

Obviously, v is correct. But u and g(s) are wrong.
This is the code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
//Includes
using namespace std;

char *entry, text, let[501], choice[2];
unsigned int i = 0, let2[501],result[501], shift,len;

void code ()
{
entry = new char [501];
cout << "Enter text to be converted:  ";
cin.getline (entry,501, '\n');
cout << endl << endl;

cout << "Enter the Character to shift in this cipher:  ";
cin >>shift;
len = strlen(entry);
while((i < len))
{	(i = i+1);
	let[i] = entry[i];
	if (let[i] == 'a'|| let[i] == 'A')
	{let2[i] = 1;}
//Similar if statements for all alphabets!!
result[i] = let2[i] + shift;
}
while ((i > 0))
{
	i--;
	if (result[i] == 1)
	   {text = 'a';}
//Again similar if statements
	cout << text;
		
};
delete [] entry;
}

int main()
{
code();
return 0;
}


I needed help with two things.
One:
How do I denote a space for those if statements?i.e. If I enter two words, the code must also return a space in the solution.
Two:
Where's the problem in my code? (and yes I know that the code is overly and needlessly complex)
Thanks in advance.

EDIT: And yes. I forgot to add, I haven't added the modulo operator to the final step, but I will after it begins to work properly!!
Last edited on
Hopefully you understand that having a condition for every letter of the alphabet is redundant. You should take a look at http://en.wikipedia.org/wiki/ASCII which can help you clean your code and better understand what you're trying to do. Also, for loops will be better in this case since you are updating i inside your loops.
Thanks . That Extremely shortened the code.
It now gives another form of error.
Firstly, This is the code:
len = strlen(entry);

1
2
3
4
5
6
7
8
while((i < len))
{		
		
i++;						
result[i] = static_cast <int>(entry[i])+ shift;			
cout << static_cast <char> (static_cast <int>(entry[i])+ shift);

}


Everything else is the same as before (I just removed the unnecessary variables and all the If statements).

Now for the error:

As for using for rather than while, it stops increasing i after it becomes 1 even if [tt]len[/ tt] is larger than 1. So I simply used while;

The error I now get is such: I enter ABC for encryption. I set the shift to 1 and I get the result CD(a Smiley, If shift is larger, maybe a heart or some other figure)

Any help?

Last edited on
1. Is there any good reason you aren't using C++ strings?
2. I would really suggest rewriting your program. Use less variables, less redundant code, etc.
3. You would definitely need to check inside the loop if the character entry[i] is above 'z' - shift so you can set the result[i] to be 'a' + shift (same for A and Z).

Back to suggestion #2, Consider all the important special cases such as when the character isn't a letter.
1. Yes. As far as I know, there is no way to separate each character in a C++ string. It is possible to do so with arrays.

This is the smallest code I could get:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
#include <iostream>
#include <sstream>
#include <string>

using namespace std;

char *entry;
unsigned int i = 0, result[501], shift,len;



void code ()
{
entry = new char [501];
//
//Getting the Text and Code Set:
//
cout << "Enter text to be converted:  ";
cin.getline (entry,501, '\n');
cout << endl << endl;

cout << "Enter the Character to shift in this cipher:  ";
cin >>shift;

//
//Getting the Length of the Text
//
len = strlen(entry);
	
	
while((i < len))
{		
		
i++;						
result[i] = static_cast <int>(entry[i])+ shift;			
cout << static_cast <char> (static_cast <int>(entry[i])+ shift);
		
	
}
delete [] entry;
}
			

int main ()
{
code ();
//Termination
cout << endl << endl << endl;
return 0;
}


Is this better?
And I didn't understand what you told in #3.
1. They are dynamic and easier to use [http://cplusplus.com/reference/string] Also, why have you included the header?
2. I'm not concern about the length of the code but about the code being clear.
3. Please consider the following special cases which need to be dealt with differently:
---- The character isn't a letter (no need to change it).
---- The character, when shifted, is out of the range of letters.*

* Please consider the following example: You want to shift y (121 in ASCII) 4 times, it will become } (125 in ASCII) but you actually wanted c (99).
1.As for the Header, I forgot to remove it after the first having the idea of using it.But I couldn't find any way to separate each character to change it, and then put it back as one. So can it be used for this purpose?If yes,how?
2.The code Seems pretty clear to me. The only unclear part seems the one where I change entry[i] + shift from char to int and back to char.
3. I did think of that. That is why I asked how to denote the space. I would have used it to return a space. But now using ASCII, I think I can put limits around it changing it.
I have thought of the final case where we are moving a character out of its range (as you gave in the example). I will use if statement and || operator to put up two conditions (if it is greater than say 90, it will subtract 26 from it, same for the other set of small characters).
1. Separate characters can be access using the [] operator.
2. The code is okay now.
3. Seems like you got the idea, have you tried implementing that? how did it go?
1.So to access the first character, I will type, string[0], and so on for every character?

@3.I didn't try to implement it yet, because I was not even getting the correct result for the simplest cases (like moving A 2 blocks...)
But I found the error (after a lot of trial and error!) The problem was in line 34. I should have written it at the end of the loop. Because I hadn't, it began with entry [1]., Ignoring the first character, and for some reason printing an extra character at the end.

I will try it now.

Thanks Again.

I now implemented it. It worked for spaces and similar characters. But it doesn't work for anything else.
Here's the code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
if (static_cast <int>(entry[i]) < 41 || static_cast <int>(entry[i]) > 122)
{
	cout << entry[i];
}
if (static_cast <int>(entry[i]) > 40 && static_cast <int>(entry[i]) < 91)
{
	cout << static_cast <char> (static_cast <int>(entry[i])+ shift);
}
else if (static_cast <int>(entry[i]) > 90 && static_cast <int>(entry[i]) < 97)
{
	cout << (entry[i]);
}
else if (static_cast <int>(entry[i]) > 96 && static_cast <int>(entry[i]) < 122)
{
	cout << static_cast <char> (static_cast <int>(entry[i])+ shift);
}
else if (static_cast <int>(entry[i] + shift) > 90 && static_cast <int>(entry[i] + shift) < 97)
{
	cout << static_cast <char> (static_cast <int>(entry[i]) - 26);
}
else if (static_cast <int>(entry[i] + shift) > 122)
{
	cout << static_cast <char> (static_cast <int>(entry[i]) - 26);
}

What I am trying to o here is :
if the original entry (entry[i] an an integer) is not an alphabet, it will remain same.
If it is a character, it will print the shifted one in place.
If the shifted character ([code]static_cast<int>(entry[i]) + shift) is not a character, it will subtract 26. But it doesn't work, and in case of Z, it gives
[
.
Any help?
You have a lot of static casting when it isn't actually needed. I think it would be easier if I provide an example of encoding a character (since this is the problematic part):
1. Lets agree that we search for characters we want to shift first (letters). We'll check if they can go "out of range" and deal with the possible cases:
1
2
3
4
5
if (ch >= 'A' && ch <= 'Z') { // The most naive way to do this is also the simplest.
	if (ch+shift > 'Z') // will go "out of range" when shifted, solution as discussed.
		return (char)(ch-26+shift);
	return (char)(ch+shift); // will not go "out of range" when shifted.
}

2. Obviously in this example I only included characters from A to Z, so you'll have to add a to z (it would be easier in a separate if).
3. As for character that did not match our search - they aren't letters so they don't need to be shifted. return ch; is enough as the last case.

I wrote it as a function [char shift(char ch, int shift)] but you can do it as part of your previous code. Hopefully, this is enough to solve the problem - I avoided posting the full solution so you still have to grasp the concepts behind dealing with characters.
I tried out your solution and it is working almost perfectly.
I get an A if my text has a space. If it doesn't have a space, I don't get anything at all.
But if I included (actually I did so unintentionally) cout << static_cast <char> (static_cast <int>(entry[i])+ shift); before the following code in the code function, it worked almost perfectly.
The only error occurred when the character went out of the range or was a special character, I would get the special character + shift and then the same character, and in case of letters, The out of range character and the the supposed to be character (.e., For text == Z, and shift == 1, I would get
[A
)
This is my code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
if (entry[i] >= 'A' && entry[i] >= 'Z')
{
 if (entry[i] + shift > 'Z' && entry[i] < 'a')
 {
	cout << static_cast <char> (static_cast <int>(entry[i]) -26 + shift);
 }
 else if (entry[i] + shift > 'A' && entry[i] + shift < 'Z')
 {
	cout << static_cast <char> (static_cast <int>(entry[i]) + shift);
 }

}

if (entry[i] >= 'a' && entry[i] >= 'z')
	{
 if (entry[i] + shift > 'z' && static_cast<int>(entry[i]) < 126)
 {
	cout << static_cast <char> (static_cast <int>(entry[i]) -26 + shift);
 }
 else if (entry[i] + shift > 'a' && entry[i] + shift < 'z')
 {
 	cout << static_cast <char> (static_cast <int>(entry[i]) + shift);
 }
}
if (static_cast<int>(entry[i]) >= 32 && entry[i] < 'A')
	cout << (entry[i]);	


Initially I used else if and else. I also tried using it in a function, but they all gave the same result.
In lines 1 and 14 you should check if the characters is between ('a' < ch < 'z' OR 'A' < ch < 'Z'). Take a second look at the condition in my example. Still, you are checking too much and casting too much (you don't need to cast it to int before you cast it to char and the static casting isn't necessary).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
char shift(char ch, int shift) {
	if (ch >= 'A' && ch <= 'Z') {
		if (ch+shift > 'Z')
			return (char)(ch-26+shift);
		return (char)(ch+shift);
	}
	// The same, for a different range of characters.
	if (ch >= 'a' && ch <= 'z') {
		if (ch+shift > 'z')
			return (char)(ch-26+shift);
		return (char)(ch+shift);
	}
	return ch; // NOT A LETTER! It is that simple.
}

This is how I will implement the shifting of one character, this isn't the neatest solution but this is really simple and it does work.

There is, however, a special case for which this won't work the expected way - if the shift is negative. If you do want to include this option think about the possibility a character will go "out of range" backwards.
Thanks a lot!!!!!!!
I feel so foolish, overlooking such an obvious flaw!! I had intended it to be the opposite signs (a range, greater than a but less than b) And since I copied the statement and changed the main characters, I got the code wrong in both the places!

As for the special case, I have decided to add instructions asking the user from not entering any negative value.
And just in case, I will add if statement that will state that the shift can't be negative, and will repeat the function.

I had another thing in my mind for using negative terms. I intend to make an decryptor. It will present the user with all possible statements (looping shift from -1 to -26 and a very similar code)

Thanks again.
Topic archived. No new replies allowed.