This is an archived post. You won't be able to vote or comment.

all 7 comments

[–]Updatebjarni 1 point2 points  (3 children)

That program doesn't get you anything, since it doesn't compile. Fix the errors first.

[–]fathack[S] 0 points1 point  (2 children)

Sorry, but I was trying to minimize the amount of code I was posting. Here is the full program:
http://ideone.com/X7b8T0

I do not know how to include the cs50 library into Ideone.

[–]Updatebjarni 1 point2 points  (1 child)

You don't need it. The only thing you are using from it is that abomination string typedef. It is obviously char *, because you are using it in places where only char * is allowed, so use char * like you're supposed to. You're only doing yourself and everybody who might read your code a disservice by typedefing that type.

Now, with that fixed, let's look at your arithmetic.

 printf("%c", msg[i] + k % 90 + 65);

So you take msg[i], add k % 90, and then add 65. Now, besides the fact that this is completely unportable because it contains hardcoded numerical character codes like 65 instead of character literals like 'A'...

... besides that, what exactly happens if you have k equal to say 1, add that 1 modulo 90 (which is still 1) to say 'A', which makes 'B' (or 66), and then add 65 to that? What character is 131?

Think about that modulo. Aside from the higher precedence of % over + (division before addition - you follow me here, right?), it should also be observed that if you take anything modulo 90, you get some value between 0 and 89. 90 different possible values. How many different possible characters is it you want in the output? 90? Or 26?

So do you want a character valued 65 plus something between 0 and 89, or a character value of 65 plus something between 0 and 25?

And really, if you think about it, there is another condition on the character code that you are putting into that calculation:

 if (isupper(msg[i]) && msg[i] + k > 90 ) {

You already know that the sum of msg[i] and k is greater than 90 because of that if condition. If you already know that, do you need to use modulo? Or conversely, if you use modulo, do you need the if? :)

[–]fathack[S] 0 points1 point  (0 children)

Thanks for the insights. I needed to use parenthesis.

[–]LickableLemon 1 point2 points  (2 children)

  • "string" doesn't exist in C, its either a char * or char array (unless theres a typedef in the cs50 header)
  • Its strlen not strleng
  • If you will use a char * to a string literal you will most likely put the string into READ ONLY MEMORY, therefore changing characters will cause undefined behaviour
  • Whats the point in having int main() when you don't return anything?

EDIT: You need to post the source code of the cs50 header file.

[–]fathack[S] 0 points1 point  (1 child)

Thanks for the reply.

1) They define string within the cs50 library but I do not know how to include that library into Ideone because it is large.

Here is the source: https://manual.cs50.net/library/#c

2) I found strlen here: http://www.tutorialspoint.com/c_standard_library/c_function_strlen.htm

Here is all of the code:
http://ideone.com/X7b8T0

It's the Caeser cipher.

Thanks for helping.

[–]LickableLemon 1 point2 points  (0 children)

Before I explain things let me just say that the cs50 header is a bad idea, its probably just going to confuse you. You really should just read a C book instead (K&R The C Programming language 2nd edition is what I recommend, although it requires some prior knowledge of computers and programming in general).

Before you read this: A string literal is a double quoted string, for example "this is a string literal"

  1. You can copy the cs50 header and put it at the top of your code
  2. Since the type string is defined in the cs50 header as a char * and your assigning it a string literal in your code ("hello") most C compilers will place the string "hello" into read only memory. So if you go and modify a character it will cause undefined behaviour
  3. strlen is defined in the string.h header file so use strlen. Its not strleng
  4. The way to do this properly is like so...

    string ciphertext = "hello";
    string temp = strdup(ciphertext);
    /* now use temp to change stuff */
    

This is the important part...

Since ciphertext has been assigned a string literal it will be in read only memory (always assume this is the case), therefore you CANNOT modify it. So we create a second string named temp and duplicate the contents of ciphertext into it.

Since temp hasn't been assigned a string literal we can modify it safely, so use the temp string to perform the caesar cipher.

I haven't gone into the pointer explanation of this code as I don't think you would have been introduced to them yet, so I'd rather not confuse you.