1

I'm trying to create a "Caesar cipher". I was hoping my code was finally complete however I ran into some errors while trying to run the code. I'm a complete beginner with C so I suspect I'm just not correctly calling a function BUT there will probably be more issues with my code. Here it is:

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

bool only_digits(string s);
char rotate(char c, int n);

int main(int argc, string argv[])
{
    if ((argc == 2) && only_digits(argv[1]) == true)
    {
        //convert string from argv[1] into int
        int n = (atoi(argv[1]));
        string plaintext = get_string("plaintext: ");
        rotate(plaintext,n);
        printf("ciphertext: %s \n", plaintext)
    }
    else
    {
        printf("usage: ./caesar key\n");
    }
}

bool only_digits(string s)
{
    size_t n = strlen(s);
    for (size_t i = 0; i < n; i++)
    {
        if (!isdigit(s[i]))
        {
            return false;
        }
    }

    return true;
}

char rotate(char c, int n)
{
    if (c >= 65 && c <= 90)
    //establish capital letter
    {
        while (n>0)
        {
            (c++ && n--);
            if (c > 90)
            {
                (c ==65)
            }
        }
        return c;
    }
    if (c >= 97 && c <= 122)
    //establish lowercase
    {
        while (n>0)
        {
            (c++ && n--);
            if (c > 122)
            {
                (c == 97)
            }
        }
        return c;
    }
    if (c >= 48 && c <= 57)
    //establish number
    {
        while (n>0)
        {
            (c++ && n--);
            if (c > 57)
            {
                (c == 48)
            }
        }
        return c;
    }
    else
    {
        if (n=0)
        {
            continue
        }
    }
}

I was getting multiple errors, some of which I already resolved. This is the latest error:

caesar.c:17:16: error: incompatible pointer to integer conversion passing 'string' (aka 'char *') to parameter of type 'char'; dereference with * [-Werror,-Wint-conversion]
        rotate(plaintext,n);
               ^~~~~~~~~
               *
caesar.c:8:18: note: passing argument to parameter 'c' here
char rotate(char c, int n);
                 ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
2 errors generated.
make: *** [<builtin>: caesar] Error 1
dariocodes
  • 133
  • 1
  • 1
  • 7
  • 2
    You need to drop CS50 and it's incredibly stupid "string" typedefs. Then rewrite the rotate function so that it takes a character pointer as parameter. – Lundin Aug 27 '22 at 18:50
  • @Lundin I appreciate your feedback. I'm completely new to coding and heard a lot of good stories about CS50. Thus, why I decided to take it up and start following their lessons. I am curious about what direction you would point me in to start learning instead of following CS50. – dariocodes Aug 27 '22 at 18:56
  • 1
    I would recommend to pick up a good C book instead. Online tutorials have diverse quality. – Lundin Aug 27 '22 at 19:00
  • 1
    Just understand CS50 hides a few things from you, foremost there is no `string` type in C, that's just a typedef CS50 does to `char *`. (which is a confusing introduction to C) Nonetheless there is good educational value in the CS50 project. With Caesar -- make sure your shift is in the right direction, see [Caesar cipher - Wikipedia](https://en.wikipedia.org/wiki/Caesar_cipher) CS50 never learned [Is it a good idea to **typedef** pointers?](http://stackoverflow.com/questions/750178/is-it-a-good-idea-to-typedef-pointers). – David C. Rankin Aug 27 '22 at 19:01
  • @Lundin: When I first encountered CS50, my first reaction was exactly the same as yours. I did not like CS50 hiding a `char *` behind the typedef `string`. However, after taking a closer look at CS50, my opinion now is that it is meaningful to do this for the first few lessons. The true nature of strings is revealed to the students in [week 4](https://cs50.harvard.edu/x/2022/weeks/4/) of CS50, when the students learn what a pointer is. [continued in next comment] – Andreas Wenzel Aug 27 '22 at 19:36
  • @Lundin: [continued from previous comment] Before learning about pointers, students are supposed to use the typedef `string` instead of `char *`, so that they can deal with string input without first learning what a pointer is. Of course, the disadvantage of this is that it is harder for students to understand the error messages emitted by the compiler. – Andreas Wenzel Aug 27 '22 at 19:36
  • As far as I can tell, you accepted one of the answers, but did not upvote that answer. Although you are free to accept and upvote as you wish, I suspect that you are unaware that you can do both, because accepting but not upvoting is an uncommon thing to do for someone who has the [upvote priviledge](https://stackoverflow.com/help/privileges/vote-up). – Andreas Wenzel Aug 27 '22 at 20:33
  • @Fe2O3: Does that mean that you are in favor of students being forced to learn pointers before being able to perform any kind of string input? Using the `string` abstraction makes it possible for students to use string input after learning about arrays, before learning about pointers. After learning about pointers, students can stop using the `string` abstraction. – Andreas Wenzel Aug 27 '22 at 21:22
  • @AndreasWenzel Students can learn array notation and that C "strings" are null terminated arrays of `char` requiring 1 byte more than expected. Many newbie questions here demonstrate lack of appreciation of how memory storage works. Without that, students will stumble and fall farther down the path... Why make 'em wait? Additionally, hiding pointer types in a typedef is bound to come back to bite the coder... Bad habits shouldn't be taught by Harvard, imho... – Fe2O3 Aug 27 '22 at 21:28
  • @AndreasWenzel THIS CS50 student wrote `if ( !isdigit( s [ i ] ) )`... So the question becomes, "when is it a magical string, and when is it an array???"... Teaching confusion... – Fe2O3 Aug 27 '22 at 21:42
  • @Fe2O3: In [week 2](https://cs50.harvard.edu/x/2022/weeks/2/) of CS50, arrays are taught and it is revealed that a "string" is a null-terminated character array. In that week, array notation and the memory layout of a string is also taught. The only thing that is not yet taught is that the data type `string` is actually a `char *`, because pointers are only taught later in week 4. OP's assignment is from [Problem Set 2](https://cs50.harvard.edu/x/2022/psets/2/), which is intended to be done at the end of week 2. Therefore, the students should have learnt everything necessary by then. – Andreas Wenzel Aug 27 '22 at 22:41
  • @AndreasWenzel I agree with 'data hiding'. I do not agree with 'datatype hiding'... You are, of course, free to have your own take on the matter. `:)` – Fe2O3 Aug 27 '22 at 22:51
  • 1
    @Fe2O3 Given that `string` is an evil typedef for `char*`, then `string str = 'Q';` will not compile cleanly, since it would then be invalid C and a conforming compiler has to give a diagnostic message. – Lundin Aug 29 '22 at 06:22
  • @Lundin Thank you... Blinded by my prejudice... I'm deleting that comment of mine... Thank you for your clear vision. :-) – Fe2O3 Aug 29 '22 at 06:30
  • @Lundin ... however, although the CS50 'newbies' would not yet know to write this, `char q = 'Q'; string p = &q;` suggests that 'p' points to a string, when it clearly does not. (After all, `int`s can become `long`s without any fuss...) Gotta mind those 'P's & 'Q's `:-)` – Fe2O3 Aug 29 '22 at 06:39

2 Answers2

1

The error is pretty self-explanatory. The rotate function expects a char but you are providing a string which is just a typedef for char *.

Given the way it's used, I suspect you want to rotate each character in the input string using a loop.

A few other notes:

char is a numeric type. Rather than using 97 in your code, for instance, you can use the char literal 'a'.

It's common convention to put any conditions that would cause termination of your program upfront and return with a non-zero exit code.

int main(int argc, string argv[])
{
    if (argc != 2 && !only_digits(argv[1]))
    {
        printf("usage: ./caesar key\n");
        return 1;
    }

    // convert string from argv[1] into int
    int n = (atoi(argv[1]));
    string plaintext = get_string("plaintext: ");
    rotate(plaintext,n);
    printf("ciphertext: %s \n", plaintext);

    return 0;
}
Chris
  • 26,361
  • 5
  • 21
  • 42
  • It would normally be self-explanatory, but CS50 insists on confusing its poor students to the max by hiding pointers behind typedef instead of teaching C. Really bad class, stay clear of it. – Lundin Aug 27 '22 at 18:51
  • That "bail out early" conditional should be using "||" instead of "&&"... – Fe2O3 Aug 27 '22 at 23:03
1

The function rotate takes a single character as an argument and returns the corresponding rotated character.

However, in the line

rotate(plaintext,n);

you are attempting to pass an entire string (i.e. several characters) instead of a single character. You are not allowed to do this.

If you want all characters in the string to be rotated, then you must call the function rotate once for every character, by using a loop:

printf( "ciphertext: " );

int len = strlen( plaintext );

for ( int i = 0; i < len; i++ )
{
    char c;

    c = rotate( plaintext[i], n );

    printf( "%c", c );
}

printf( "\n" );
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39