[n00b]Caesar's Cipher

Laika1986

Hey guys im pretty new to C programming.Apologies i forgot to mention that this was C code in the thread title. I am writing a program based on caesar's cipher. Basically the program takes a string from the user and encrypts it by a user defined amount of chars to form a new string.

The problem i am encountering is when 'z' for example is entered i need to send the char back to the start of the alphabet. I have tried numerous ways of doing this but nothing seems to work. At the moment it just continues through the ASCII code so a V produces ]. I hope i have explained this properly.

#include <stdio.h>
#include <string.h>

int main (int argc, char *argv[])

{

	char str[80];//declares array str of value 80

	int i,x,rem;
	x= atoi(argv[1]); 
	
	printf("Please enter a string: ");
	fgets(str,80,stdin);//prompts user and stores value

	for (i=0; i<=strlen(str); i++){//loop to run through string

		if (str[i]>=65 && str[i]<90){

			str[i]=str[i]+x;//produces uppercase equivalent 
			
			putchar(str[i]);
	
			}
			
		if (str[i]+x>=91 && str[i]+x<97){
			
			rem=str[i]%x;
		
			str[i]=str[65]+rem;
			
			putchar(str[i]);}
			
		if (str[i]>=97 && str[i]<122){
		
			str[i]=str[i]+x;
			
			putchar(str[i]);}
		
		if (str[i]+x>122){
		
			rem=str[i]%x;
			
			str[i]=str[97]+rem;
			
			putchar(str[i]);
			
		}

Question1: why when str is greater than 122 does it not proceed the steps in the if statement?

Question2: I believe i am close to the end of this question but i think my second and fourth if statements are holding me up. Am i correct?

Any help would be greatly appreciated

Procasinator

I don't have a C++ compiler handy, but if I had to guess at a problem I would think the Modulo operator (%) could be causing you some problems.

You see, a char is generally a byte. We think of standard ASCII as values between 0-127, with extended characters up to 255.

When we are working with characters, this is fine. There is a mapping between a distinct 8-bit sequence.

For ASCII Char 127 we would have the binary representation (01111111)
For ASCII Char 128 would have the binary representation of (10000000)

And when we are dealing with char arrays, this is all fine.

Now, when we are using the % operator, we are treating that char as a numerical byte.

So let's say we have the char 128, and we % it. So, a user passes in say 2, our mod looks like this:
128 % 2 = 10000000 % 00000010

Now, we know that 128 % 2 == 0. However, do you know what -1 % 2 equals? Why do I say -1: because there is good chance on your platform a character is treated with twos complement. That is, the first bit is used to denote sign (0 is positive, 1 is negative).

In C++, the result of modulus with negative operands is implementation defined.

You need to specifically treat the str as an unsigned char.

TL;DR char over 127 might be treated as a signed char, and seen as negative. How the % treats negative operands is defined by the implementation.

Aswerty

First thing I'd say is that your first and third if statements should have <= 90 as opposed to < 90 and <= 122 as opposed to < 122 because your currently not handling z and Z correctly.

Secondly I think your going down the wrong direction by using modulo. From what I can see the loop around functionality doesn't make sense (I could be wrong though because modulo fries my brain). For a problem such as a ceasar cipher I would consider a loop around to be best implemented by:

str[i]=str[i]+x;

if(str[i] > 122)
{
  str[i] = str[i] - 26
}

And using the same thing for uppercase letter just changing the if statement. The -26 is of course the size of the alphabet.

carveone

Just a small point. It can sometimes be easier and clearer to use a look up table. You can create a table like this for rotate by + 2:

char table[] = "cdefghijklmnopqrstuvwxyzab"

if ((ch >= 'a') && (ch <= 'z'))
ch = table[ch - 'a'];

I'm just showing you this because it is used so often. You subtract 'a' (or 'A') from the letter to give an offset in ascii. Then you can look the number up in a table or do calculations on it without wrecking your head.

Here's another example. I've broken up the lines to make it clearer:

if ((ch >= 'A') && (ch <= 'Z'))
{
    ch = ch - 'A';    /* offset. Now 0 to 25. */
    ch = (ch + 13) % 26;        /* Rot 13 */

    ch = ch + 'A';    /* Back to a letter */
}

See? Then it's just maths. Modulo arith just keeps a number within a certain bounds.

Hope this helps

Procasinator

Well, the modulus is useful for for the wrap around. But a couple of the techniques are wrong.

For instance, this block:

if (str[i]>=65 && str[i]<90){

			str[i]=str[i]+x;//produces uppercase equivalent 
			
			putchar(str[i]);
	
			}

Reads: If character is upper-case, add user-defined x. But your comment says you "produces uppercase equivalent". That code is only applied to upper-case characters though. Is it that you really just want to translate lower-case characters to upper-case or vice-versa?

Regarding %, I listed some problems earlier. But really, you are using modulus wrong, and can avoid those issues altogether.

Look at this article: http://en.wikipedia.org/wiki/Caesar_cipher

It has a formula, which is pretty much:

e = (x + n) % 26;

Where e stands for encrypted value, x is your character (not to be confused with the x variable in your code, which actually refers to the shift amount) and n is how much you shift by. The mod then wraps it around if it overflows from 26. So, say:
x = 25 (maps to letter z)
n = 5

Then (x + n) = 30.
30 % 26 = 4

4 maps to letter e.

So, your steps are:

1. Get a mapping of your ASCII character to a number between 0 (letter a) and 25 (letter z).
   
2. Using above formula, translate the character to it's shifted value (between 0 - 25 again).
   
3. Translate your mapping to the alphabet back to the corresponding ASCII value. Make sure to account for if you have an upper-case or lower-case character if you want to preserve case.

carveone

Note that because ascii is so regimented you can play horrible games. It's kinda non portable because Unicode is prevalent now...

A is 0x41 (01000001) and a is 0x61 (01100001). The difference is bit 5. So

char small;

small = ch & 32;       /* the value of bit 5 - 1 or 0 */

ch = ch & (~small);  /* same as toupper() - clear bit 5 */

ch = ((ch >= 'A') && (ch <= 'Z') ? ((ch - 'A' + 13) % 26 + 'A') : ch);

ch |= small;

Ok. That's a bit nasty - all I've done is reduce the comparison set to A to Z by converting to upper case and remember what it was before. It was very common to toggle uppercase by flipping bit 5.

carveone

Procasinator wrote: »

So, your steps are:

1. Get a mapping of your ASCII character to a number between 0 (letter a) and 25 (letter z).
   
2. Using above formula, translate the character to it's shifted value (between 0 - 25 again).
   
3. Translate your mapping to the alphabet back to the corresponding ASCII value. Make sure to account for if you have an upper-case or lower-case character if you want to preserve case.

Yes. That. What he said

Laika1986

Well lads sorry im only getting on here now. Thanks for all your comments and patience im gonna work through them now see what i can learn!

@procrastinator- Thanks for your reply to clear up the confusion with my uppercase comment, i intended to delete that as it was from a previous program. Also your first post was excellent it really explained properly using chars as i have struggled with them since the start!
Thanks for the links and the formula!

@aswerty-thanks for the input silly little mistake really!

@carvoene-once again value input!i will probably change it to work out of an array!

Now to work!

Laika1986

Hey guys i just typed this out i said id make sure i am on the right track first!

char capstable[]=ABCDEFGHIJKLMNOPQRSTUVWXYZ
char table[]=abcdefghijklmnopqrstuvwxyz


if ((ch >= 'A') && (ch <= 'Z'))

	int e=(i+n)%26
	
	ch=ch+e
	
if ((ch >= 'a') && (ch <= 'z'))

        int e=(i+n)%26
	
	ch=ch+e

Aswerty

You're roughly on the right track but you were roughly on the right track on your first post. Procrastinators 3 steps are a good guide of what you need to do. Write the code to implement step one and check that that works, then write some more code to implement step two and check that that works, then write the code for step three and you should be sorted. When you're writing code it is good to break your problem down into little steps and then implement them while ensuring each stage works as expected.

If you cut you code in the OP back to this you are nearly finished step 1.

#include <stdio.h>
#include <string.h>

int main (int argc, char *argv[])

{

	char str[80];//declares array str of value 80

	int i,x,rem;
	x= atoi(argv[1]); 
	
	printf("Please enter a string: ");
	fgets(str,80,stdin);//prompts user and stores value

	for (i=0; i<=strlen(str); i++){//loop to run through string

		if (str[i]>=65 && str[i]<90){

			// convert str[i] to a value between 0 and 25
	
			}
			
		if (str[i]>=97 && str[i]<122){
		
                       // convert str[i] to a value between 0 and 25

			}
		

Laika1986

Aswerty wrote: »

You're roughly on the right track but you were roughly on the right track on your first post. Procrastinators 3 steps are a good guide of what you need to do. Write the code to implement step one and check that that works, then write some more code to implement step two and check that that works, then write the code for step three and you should be sorted. When you're writing code it is good to break your problem down into little steps and then implement them while ensuring each stage works as expected.

If you cut you code in the OP back to this you are nearly finished step 1.

#include <stdio.h>
#include <string.h>

int main (int argc, char *argv[])

{

	char str[80];//declares array str of value 80

	int i,x,rem;
	x= atoi(argv[1]); 
	
	printf("Please enter a string: ");
	fgets(str,80,stdin);//prompts user and stores value

	for (i=0; i<=strlen(str); i++){//loop to run through string

		if (str[i]>=65 && str[i]<90){

			[B]i=atoi(str);[/B]// convert str[i] to a value between 0 and 25
                        
                       
	
			}
			
		if (str[i]>=97 && str[i]<122){
		
                      [B]i=atoi(str);[/B] // convert str[i] to a value between 0 and 25

                      

			}
		

thanks for your reply i hope this isnt coming across as lazy im trying hard its just getting my head around it! i just have one question, can i use the variable i here as i have done?

Aswerty

You wouldn't usually assign a value to i because it is your for loop counter. Assigning a value would generally break the loop on the next iteration or put the program into a continuous loop. Typically the counter is used to index an array just like you were doing with str.

I never actually have come across the function atoi(), I would just do the following in this case to turn the individual character being looked at into an integer between 0 - 25.

int j = str[i] - 65 // for upper case

int j = str[i] - 97 // for lower case

carveone

Sorry if I confused you with table lookups and the like. Drive by coding

Let me drive by some more and look at the code:

for (i=0; i<=strlen(str); i++){//loop to run through string

Minor point - you are running strlen on each loop iteration. Rather inefficient. Calculate it once in advance. Plus you are running off into the null terminator on the string.

if (str[i]>=65 && str[i]<90){

You are asking to make an error doing that (which you have done) and it's unclear. Remember that there is no difference between 'A' and 65. They are both ints. Which may be surprising to you - yes, 'A' is an int, not a char.

i=atoi(str);// convert str[i] to a value between 0 and

That's not right. atoi takes a string like "1234" and converts it to a number like 1234. Plus you are reusing the loop iterator, i. Can't do that!

Make it clearer to yourself by doing something like:

ch = str[i] - 'A';

carveone

My code section in post 4 pretty did everything you need to do btw!

PS: Not to aggravate you more, but can the user enter any number to rotate by?
Like 20, 200, -3?

Yeah, these are the edge cases, the one's programmers spend the most time on.

Laika1986

Hey guys I really hope this is on the right track im pretty sure it is but iv been wrong before!!!

#include <stdio.h>
#include <string.h>

int main (int argc, char *argv[])

{

	char str[80];//declares array str of value 80
	char capstable[]="ABCDEFGHIJKLMNOPQRSTUVWXYZ";
	char table[]="abcdefghijklmnopqrstuvwxyz";
	
	int i,n,x,e;
	n=atoi(argv[1]); 
	
	printf("Please enter a string: ");
	fgets(str,80,stdin);//prompts user and stores value

	for (i=0; i<=strlen(str); i++;){//loop to run through string
	

		if (str[i]>=65 && str[i]<=90){

			x==atoi(str);
			e=(x+n)%26;
			putchar(capstable[e]);
			}
	
		if (str[i]>=97 && str[i]<=122){
		
			x=atoi(str);
			e=(x+n)%26;
			putchar(table[e]);
			
			}
			
			}

When i compile now and enter a string it will only return f or B regardless of what i enter so that would lead me to assume that its not taking in the variable as hoped?

Laika1986

carveone wrote: »

My code section in post 4 pretty did everything you need to do btw!

PS: Not to aggravate you more, but can the user enter any number to rotate by?
Like 20, 200, -3?

Yeah, these are the edge cases, the one's programmers spend the most time on.

Ha i don't want to aggravate ye more than anything!Ye have been great help. No the number to rotate must come from the command line

carveone

You're combining a whole bunch of different things together I think. Hope this isn't against the rules but I had 5 mins to spare and knocked something up. It's a bit rushed but try it out. Note a few points like using sizeof(array) instead of "80"...:

Yeah - giving you the answer isn't learning you nothing I guess

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int main (int argc, char *argv[])
{

	char str[80];    //declares array str of value 80

	int rot, len, i;


	if (argc < 2)
	{
		fprintf(stderr, "Too few arguments. Enter a number to rotate by.\n");
		exit(-1);
	}

	rot = atoi(argv[1]);

        if (rot < 0 || rot > 26)
	{
		fprintf(stderr, "Argument should be between 0 and 26.\n");
		exit(-1);
	}
	
	printf("Please enter a string: ");
	fgets(str, sizeof(str), stdin);   //prompts user and stores value

        len = strlen(str);

	for (i = 0; i < len; i++;)
	{
		//loop to run through string
	

		if (str[i]>='A' && str[i]<='Z')
		{
			str[i] -= 'A';
			str[i] = (str[i] + rot) % 26;
			str[i] += 'A';
		}
	
		if (str[i]>='a' && str[i]<='z')
		{
			str[i] -= 'a';
			str[i] = (str[i] + rot) % 26;
			str[i] += 'a';
		}
	}

	printf("Resulting string: %s\n", str);

        exit(0);

}

Aswerty

The piece of code that stands out for me is:

x==atoi(str);
e=(x+n)%26;

Firstly a == is only used to check if two variables are equal to each other, a typo I'm assuming but it will screw things up. Secondly I don't know why your are trying to convert the entire string into an integer. You should only be dealing with one character at a time. Try replacing:

x=atoi(str)

with

x = str[i] - 65 // or 97 for lowercase

Laika1986

Sorry lads i seem to be all over the place. Thanks for the suggested solution however i would prefer to figure this out myself although i will be using it to learn from it! Your collective advice has been great iv posted on a number of programming boards before and have only been greet with condescending advice so thanks!

@carveone-Although i understand what is happening in this code i am unfamiliar with the expressions '-=' and '+='. do they mean greater than or equal to?

for example could this code

if (str[i]>='A' && str[i]<='Z')
		{
			str[i] -= 'A';
			str[i] = (str[i] + rot) % 26;
			str[i] += 'A';
		}
	
		if (str[i]>='a' && str[i]<='z')
		{
			str[i] -= 'a';
			str[i] = (str[i] + rot) % 26;
			str[i] += 'a';
		}

be written:

if (str>='A' && str<='Z')
{
str <= 'A';
str = (str + rot) % 26;
str >= 'A';
}

if (str>='a' && str<='z')
{
str <= 'a';
str = (str + rot) % 26;
str >= 'a';
}

Thanks again

Procasinator

carveone wrote: »

My code section in post 4 pretty did everything you need to do btw!

PS: Not to aggravate you more, but can the user enter any number to rotate by?
Like 20, 200, -3?

Yeah, these are the edge cases, the one's programmers spend the most time on.

If he using the e = (n + x) % 26, it won't matter for positive numbers.

For negatives, something like this should work (before using above formula):

if (n < 0) {
    n = 26 - (abs(n) % 26);
}

Which will turn say -3 to 23 and -51 to 1 and -2011 to 17.

carveone

Procasinator wrote: »

If he using the e = (n + x) % 26, it won't matter for positive numbers.

For negatives, something like this should work (before using above formula):

if (n < 0) {
    n = 26 - (abs(n) % 26);
}

Which will turn say -3 to 23 and -51 to 1 and -2011 to 17.

I was trying to broadly hint that a negative number would mess things up!

But yes, your example would be the correct way of doing it. Convert the minus number into the corresponding positive one.

I relented and posted code that does the check at the beginning. I was still too lazy to do the conversion though.

OP: When coding you always have to be aware that where you expect "foo" someone will type "supercalifragalisticexpealidocious". These are the edge conditions that consume so much of your time. I personally write the algorithm down (on paper), figure out the code path, and then type it in doing all the error checking and edge conditions as I go. This generally doubles the size of the code!

Aswerty

Laika,

The operator += and -= are just shorthands.

str[i] -= 'A'; // the same as str[i] = str[i] - 'A'
str[i] += 'A'; // the same as str[i] = str[i] + 'A'

Laika1986

She's working alright!!Looking at the code i first posted and the code i have now am i mad in thinking my logic was pretty right just my execution was over complicated?

carveone

Laika1986 wrote: »

She's working alright!!Looking at the code i first posted and the code i have now am i mad in thinking my logic was pretty right just my execution was over complicated?

You were working your way into a corner which is easy enough to do. Add x but that means I have to check for y which puts me in z but there's problems with a,b,c. Happens all the time. I've written 100s of lines of code and then a friends says "but if you just use a table here..." and you go "argh!"...

You've seen nothing until you've seen code that checks for 1000 things, one at a time. Saw one horror that had to check if a date was between April 5th and April 4th the next year (taxes and stuff). Yup - there was actually 365 lines of code - "if (month==4) && (day == 5)" .... etc... This was on a production box.

If you get step 1 right, then everything falls into place. Luckily a lot of concepts have been invented by others.

As Aswerty said, "-=, +=, *=, etc are shorthands; I just slapped some code down and didn't think to mention that.

Good luck.