2

I have created a program in C. The program is a version substitution string. I have analyzed the program carefully by using the printf statements. My program logic is working correctly. However I am not able to add or concatenate strings.

I am not getting the output from encryptKey function. Why is that? I have earlier being programming in JS and now trying my hand on C.

Here is my code: -

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

string encryptKey(string message);

//stores the cipher word for each alphabet
//used for encryption purposes
char cipher[26];

int main(int argc, string argv[])
{

    //Exit the program if no key is added in commandline
    if(!argv[1] || argc >= 3)
    {
        printf("Incorrect or missing command line arguments!\n");
        return 1;
    }

    //checks if there are 26 characters in the decipher key
    if(strlen(argv[1]) != 26)
    {
        printf("Need 26 characters to decipher the key\n");
        return 1;
    }

    //check all the individual characters of the key
    for(int i = 0; i < 26; i++)
    {
        //check if the characters are letters only
        if(tolower(argv[1][i]) < 'a' && tolower(argv[1][i]) > 'z')
        {
            return 1;
            printf("All characters should be letters!\n");
        }

        //checks if each letter exists once

        //load the secret key to an array
        cipher[i] = argv[1][i];
    }


    //Asks user for message
    string message = get_string("plaintext:");
    string secretMessage = encryptKey(message);

    //outputs secret message
    printf("ciphertext: %s\n", secretMessage);
}

//encrypt the message
string encryptKey(string message)
{
    string output = "";

    //loop through each alphabet
    for(int i = 0; i < strlen(message); i++)
    {
        char lower = tolower(message[i]);

        if(lower >= 'a' && lower <= 'z')
        {
            //small case starts from location 97
            int tempLocation = lower - 97;


            if(isupper(message[i]))
            {
                output += toupper(cipher[tempLocation]);
            }
            else if(islower(message[i]))
            {
                output +=  tolower(cipher[tempLocation]);
            }
            else
            {
                output += message[i];
            }
        }
    }

    return output;
}

I even tried this to join strings: - strcat(output, toupper(cipher[tempLocation]));

I get this error: -

error: incompatible integer to pointer conversion passing 'int' to parameter of type 'const char *' [-Werror,-Wint-conversion]
                strcat(output, toupper(cipher[tempLocation]));
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/string.h:130:70: note: passing argument to parameter '__src' here
extern char *strcat (char *__restrict __dest, const char *__restrict __src)
                                                                     ^
Lundin
  • 195,001
  • 40
  • 254
  • 396
killerprince182
  • 455
  • 2
  • 12
  • `output += toupper(cipher[tempLocation]);` is not _string_ appending in C. That is just adding to the pointer `output`. – chux - Reinstate Monica Aug 27 '21 at 10:11
  • I tried using strcat but I got the error posted above – killerprince182 Aug 27 '21 at 10:14
  • `strcat(output, toupper(cipher[tempLocation]));` fails becasue 1) `cipher[tempLocation])` is not a _string_ and 2) `output` points to too small a space. (`string output = "";` is 1 byte for characters) – chux - Reinstate Monica Aug 27 '21 at 10:14
  • 2
    You need to call toupper in a loop for each character, ensure that the string is null terminated, and then pass it to strcat. Generally, you can't program by trial & error, you must know what you are doing. That includes studying every function you call before you call it. – Lundin Aug 27 '21 at 10:15
  • 2
    The root of all your problems with string handling is CS50. It's a very bad class failing to teach how to use strings correctly in C. I strongly recommend you to stop following it. – Lundin Aug 27 '21 at 10:16
  • Besides the fact that plain C does not have strings, as pointed out in other comments/answers, the condition ```if(tolower(argv[1][i]) < 'a' && tolower(argv[1][i]) > 'z')``` will never be true. You should use OR operator ```||``` instead of the AND one. – xuman Aug 27 '21 at 10:31
  • @Lundin I am doing Harvard's cs50 which provides that library. – killerprince182 Aug 27 '21 at 10:48
  • @xuman I am very new to this in C. Please help me understand why && won't work? – killerprince182 Aug 27 '21 at 10:49
  • @Lundin: I disagree with your criticism of CS50. The course probably decided to make this abstraction because they determined that beginner students were having significant difficulty with string handling. That is why they only reveal the true nature of strings [at this point in lecture 4](https://youtu.be/NKTfNv2T0FE?t=2435). Therefore, the problem in my opinion is not CS50, but rather that the student has (possibly) not watched lecture 4 yet. – Andreas Wenzel Aug 27 '21 at 10:57
  • ```&&``` operator evaluates as false if its first operand is false, otherwise it evaluates as its second operand. In other words, it needs its two operands to be true to evaluate as ```true```. Since you are comparing each of the characters in your string as ```< 'a'``` and ```>'z'```, both conditions can never happen at the same time: a character can take a value less than 'a', bigger than 'z', or something in between. If you want to get an error message when the character is NOT a lower case letter, you should check that the character is lower than 'a' OR (hence, ```||```) bigger than 'z'. – xuman Aug 27 '21 at 10:58
  • Thanks @xuman I understand that now. – killerprince182 Aug 27 '21 at 11:01
  • @AndreasWenzel SO gets constantly spammed down by confused students who don't understand how to use pointers or strings, because this class teaches them to hide pointers behind typedef. We have to spend a lot of energy un-teaching CS50 and then teaching proper C programming. – Lundin Aug 27 '21 at 11:11
  • How much damage has done the `string` type in the CS50 course!!!! I'm afraid. The students (more if they have had an introduction also to C++) tend to interpret `string` as the C++ counterpart, and start thinking on appendable objects that are not present in C. – Luis Colorado Aug 27 '21 at 12:49
  • @AndreasWenzel, I'm afraid there's nothing even close enough in C to be called `string` to make the abstraction they have done in CS50. I have not followed that course, but I'm glad of never had to attend it. What `string` actually is in C is an absolute and complete misnaming, infortunate enough with the fact that many students today enter the programming route by having courses indistinctly in C and C++, and they tend to mistake C++ `string`s with CS50 definition (which is just a renaming of `char *`) – Luis Colorado Aug 27 '21 at 12:53
  • @LuisColorado: The CS50 course is designed for people who have had no prior programming experience. Without these CS50 helper functions, getting any kind of string input from the user would be impossible or at least very error-prone at the start (as `scanf` should not be used and `fgets` is complicated). After about 8 hours of lecture time in the language C, the true nature of strings is revealed to the students. I think this is handled very well by the CS50 course. – Andreas Wenzel Aug 27 '21 at 13:11
  • IMHO it's a bad habit to use names that are so common in programming literature and used for quite different things... I don't know if you treat to make me think that anybody that starts learning C will never have a look at C++ and get the different meaning of the `string` type used there? Please, argument something, as you are trying to defend CS50 as if you were a stakeholder of the academy. Despite of being CS50 and the matters taught there, I'm full (and the other people that has commented also in the same way) of seeing confused people just because they think a string is a string and... – Luis Colorado Aug 27 '21 at 13:14
  • ... not just a `char` pointer. – Luis Colorado Aug 27 '21 at 13:14
  • Let's say that I start a surgery course but I decide that my notation will be to change names of all body organs ---e.g. kidney will be used for the lungs, liver for the stomach--- and the like. I can give an excellent surgery course and nobody discusses that.... but as a professional of programming, I can say that that was a bad election, despite it comes from Harvard or MIT. – Luis Colorado Aug 27 '21 at 13:17
  • @LuisColorado: You are right that abstracting a `char *` as a `string` causes confusion. On the other hand, requiring students to do memory management when dealing with strings, before they are ready for it, also causes confusion. The CS50 course made the determination that the latter would cause more confusion, which I find understandable. And I guess that they probably were correct in making that determination. – Andreas Wenzel Aug 27 '21 at 13:27
  • As a 40+ years experienced C programmer, I've seen only students confused because they think a string should do more than a `char *` does. This on seven years answering questions here in SO and teaching C to colleagues at work. Anyway, it is not worth more discussion, from my point at least. – Luis Colorado Aug 27 '21 at 13:35
  • @LuisColorado: When I first encountered CS50's `string` data type, my first reaction was similar to yours. I asked myself: Why do they not teach proper ISO C? Why do they teach their students to use these proprietary functions instead? However, after watching CS50 myself, I now understand why they are doing this, and believe that they are doing a good job. – Andreas Wenzel Aug 27 '21 at 13:38
  • Making mistakes is human... but not correcting them marks you forever. In my opinion many students end here asking for someone to do their homework... they enter in programming teams... and they are absolutely dependent on SO. Which is not bad... but they are not actually good programmers. When I see 50 times the same question asked in a forum like this... I tend to think there's something wrong in the teaching of such a group of students. I see the problems they post, some of them are quite interesting... but not correcting that is an error... anyway. – Luis Colorado Aug 27 '21 at 13:41

1 Answers1

4

As mentioned by chux - Reinstate Monica: Strings working different in C.

Here are some hints:

  • Give output some memory:
char output[512] = { 0 };
  • Assign string value correct (be sure that output is 0 terminated string \0 at last index) f.e.:
char ch = toupper((unsigned char) cipher[tempLocation]);
strncat(output, &ch, 1);
  • Change signatures of your methods like:
// message: is a string (pointer to char[] which is a string)
// enrypted: copy created string (output) at end of the method to encrypted instead of return output
// int is the return value - 0 success, != 0 failure
int encryptKey(char *message, char *enrypted)
  • copy result to given char memory:

instead of:

string secretMessage = encryptKey(message);

do:

char secretMessage[512] = { 0 };

// no & needed ... a array is a pointer
if (encryptKey(message, secretMessage) != 0)
{
  printf("\n Something failed");
}

and at the end of your method:

strcpy(encrypted, output);

Like others commented doing the whole thing trial & error will get you nowhere.

Hope the hints are enough for you to figure out the rest.

Kraego
  • 2,978
  • 2
  • 22
  • 34
  • @chux - Reinstate Monica thanks, yes the empty one ```{}``` just works in c++ – Kraego Aug 27 '21 at 10:23
  • 1
    Note: `return output;` remains a problem - returing address of local variable. – chux - Reinstate Monica Aug 27 '21 at 10:28
  • @chux - Reinstate Monica thanks for the hints, recognized I'm a little bit rusty with my C skills ;-) – Kraego Aug 27 '21 at 10:38
  • 2
    Telling a student not to use a library needed for a course they are working on a problem for is bad advice, regardless of your opinion of the library. You could advise them the library is merely a temporarily crutch for early lessons and ought to be abandoned later. – Eric Postpischil Aug 27 '21 at 10:59
  • Removed the CS50 part from answer, cause its just my opinion. imho: it will leed to to more confusion (see compiler errors with char *) when working with other libraries as demonstrated in this question. I also didn't know thats just a temporary crutch, @Eric Postpischil thanks for that information. – Kraego Aug 27 '21 at 11:08
  • 1
    The CS50 course reveals the true nature of strings [at this point in lecture 4 of the course](https://youtu.be/NKTfNv2T0FE?t=2435). – Andreas Wenzel Aug 27 '21 at 11:16
  • 2
    thanks again for all the hints (added the ```unsigned char``` cast). – Kraego Aug 27 '21 at 11:28