1. Caesar Cypher

Hi. I am trying to make a caesar cypher.

For those that don't know what it is:

It takes a string and an integer and moves each letter in the string along by that integer. So say the string was ABCD and the shift was 1, then the new message would be BCDE. Ya dig?

Anyway, Ive almost got it complete, but there are a few bugs which I can't seem to track down:

Caesar.c:
Code:
```#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char shift(char *str, char c, int shiftnum, int length);

int main(void) {

char stringtoshift[100];
char shiftvalue[10];
char *s = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
int len = strlen(s);  /* = 62 */

printf("Enter string: ");
fgets(stringtoshift, 100, stdin);
int length = strlen(stringtoshift);

printf("Now enter the shift value: ");
fgets(shiftvalue,10,stdin);
int sh = atoi(shiftvalue)% len;

if(sh<0)
sh += strlen(s);   /* if shift is -1 for eg, its the same as shift being 61 */

int j;
for(j=0;j<length;j++) {
if(stringtoshift[j] == ' ');  /*skip spaces*/
else
stringtoshift[j] = shift(s,stringtoshift[j],sh,len);
printf("%c", stringtoshift[j]);
}

stringtoshift[j]='\0';

printf("\n");

return 0;
}

char shift(char *str, char c, int shiftnum, int length) {
int i;
for(i=0; *str!=c; str++)
i++;                                          /* i is the position we are currently in the string str */
if(i+shiftnum >= length) {

/* if you are on Y (position 51) for example
and shift is 23, then it will spill over
(a-z = 26, A-Z = 26, 0-9 = 10, total = 62)
(62-51=11, 23-11=12) so we have to move 12
places from 'a' */

shiftnum += i - length;            /* 23 - (62-51) = 23 + 51 - 62 */
for(;*str!='a';str--);                       /* reset (go back to beginning of s) */
}

for(;--shiftnum>=0;)                           /*same is for(k=0;k<shiftnum;k++) */
str++;

return *str;
}```
Running:
Code:
```\$gcc -o caesar caesar.c
\$./caesar
Enter string: HELLO THERE
Now enter the shift value: -1
GDKKN SGDQD�
\$ ./caesar
Enter string: hello
Now enter the shift value: -8
96ddgo```
These are just a few examples of unexpected output. If you could, try playing about with it yourself to see what is wrong, I would be very grateful. Thanks

2. fgets leaves the trailing newline in your buffer which you then try to convert. You probably want to replace that newline with a null prior to taking the length of the input string.

Either that, or you maybe can change:
Code:
```  for(j=0;j<length;j++) {
if(stringtoshift[j] == ' ');  /*skip spaces*/```
To this:
Code:
```  for(j=0;j<length;j++) {
if(stringtoshift[j] == ' ' || stringtoshift[j] == '\n');  /*skip spaces*/```
...so you don't attempt to convert that character.

3. It takes a string and an integer and moves each letter in the string along by that integer. So say the string was ABCD and the shift was 1, then the new message would be BCDE.
Actually, that is a transposition cipher. A Caesar cipher is a substitution cipher that creates a mapping by 'shifting' the alphabet. Still, it does appear that you are trying to implement a Caesar cipher.

Your shift() function seems overly complicated. I would expect that all you need to do is to find the position of the letter in the alphabet string, add the offset to that position, and then return the letter at that new position.

4. To solve the "spill over" problem, use the modulus operator. Your shift() function could be something as simple as this:
Code:
```char shift(char *str, char c, int shiftnum, int length) {
char *p = strchr(str, c);

return p ? str[(p - str + shiftnum) &#37; length] : c;
}```
NOTE: chars passed to the function which aren't in str won't change.

5. @itsme86:
I used the modulus operator before hand:
Code:
```int sh = atoi(shiftvalue)% len;
/* `int sh = atoi(shiftvalue)% strlen(s)` didn't work for some reason */```
@laserlight
The problem I had with this is that I had to think of a way to move around in my character string. If I used an offset, I would have still had to figure out which character was at that offset, and the result would be something similar to the above.

@hk_mp5kpdw
Thanks, I didn't know fgets() put an \n in my buffer.

Thanks for all your replies, much appreciated

6. Code:
`/* `int sh = atoi(shiftvalue)&#37; strlen(s)` didn't work for some reason */`
strlen() probably didn't work directly because strlen() returns a size_t (an unsigned integer), while len is declared as an int. I don't know why size_t doesn't work, but that's the only difference between the two lines.

Also, I don't like atoi() myself. Use strtol()!
Code:
```#include <stdlib.h>

char *str = "123";
long number = strtol(str, NULL, 0);```
 There's a reason I don't like atoi() BTW. It doesn't do any error checking. Even the manual says that it's deprecated in favour of strtol(): http://opengroup.org/onlinepubs/007908799/xsh/atoi.html [/edit]

7. Code:
`char *s = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";`
Why do you need 0123456789 ? meow. Generally caesar rots char not numbs.

Code:
```[pbqr]
pune *f = "nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM0123456789";
[/pbqr]

Jul qb lbh arrq 0123456789 ? zrbj. Trarenyyl pnrfne ebgf pune abg ahzof.```

8. WHy Do you nEED defghijklm MEow 6EnErALLy CAEsAr rots CHAr not nuMBs
Contradiction haha, I know what you meant. I just wanted to allow the user to input numbers too, no biggy. I have since added &*()/-+ etc to the string too.

9. Originally Posted by Tommo
@itsme86:
I used the modulus operator before hand:
Code:
```int sh = atoi(shiftvalue)&#37; len;
/* `int sh = atoi(shiftvalue)% strlen(s)` didn't work for some reason */```
You can't do it that early, because you don't know where the original char falls in the string yet. You have to do it in the function like I did.

10. You can't do it that early
Well I can because I have and it works nicely.

The code in the if statement in the shift() function essentially does the same thing though. Thanks

11. Originally Posted by Tommo
Well I can because I have and it works nicely.
Well, I know you can but it's unnecessary. The single modulus operation in the shift() function takes care of it. It also might obscure things because you don't retain the number that the user actually entered.

Also, the method you use for avoiding the "spill over" (which is what I was actually addressing and your modulus operation in main() doesn't do) has a couple of potential problems:
1) It's slower. A simple arithmetic operation is faster than looping through the entire string.
2) If the first character in the string ever changes from 'a' it can be easily overlooked and will break your loop that finds the beginning of the string, causing a hard-to-find bug. Especially if it was a more complex program. This kind of "solution" is often discouraged just for that reason and you'll want to avoid using code that can be broken by other areas in the program as you advance. Bad habits are hard to break.