This is the code of my program where I encrypt a text that the user gives me. this is for the CS50 course.

char* rotate(string plaintext, int k)
{
  for (int i = 0; i < strlen(plaintext); i  )
  {
    if (isupper(plaintext[i]))
    {
      printf("%c", (((plaintext[i] - 65)  k) &  65));
    }
    if (islower(plaintext[i]))
    {
      printf("%c", (((plaintext[i] - 97)  k) &   97));
    }
    else
    {
      printf("%c", (plaintext[i]));
    }
  }
  return 0;
}

That is all the program, I just want the NULL value to not appear at the end.

CodePudding user response：

You are printing the encrypted text but are not returning it. Does the caller print it? I suspect it does because of your (NULL) statement.

1. You [probably] want to put back the encrypted chars into the string (i.e. remove the printf because caller will do it).
2. Change the function return to plaintext. When you did return 0;, this was the probable source of the (NULL) being printed.
3. Don't use decimal constants. Use 'A' and 'a'
4. Doing i < strlen(plaintext) will cause the operation to have O(n^2) time because strlen must rescan the string on each iteration.
5. Better to do: plaintext[i] != 0 as this will run in O(n) time.

Here is a refactored version:

char *
rotate(string plaintext, int k)
{
    for (int i = 0; plaintext[i] != 0; i  ) {
        // get current plaintext char
        int chr = plaintext[i];

        if (isupper(chr))
            chr = (((chr - 'A')   k) % 26   'A');

        else if (islower(chr))
            chr = (((chr - 'a')   k) % 26   'a');

        // put back encrypted char
        plaintext[i] = chr;
    }

    return plaintext;
}

CodePudding user response：

return plaintext; instead of 0 (NULL).

It is misleading that you call the argument plaintext when you mutate it into ciphertext. Here is an alternative:

string rotate(string s, int k) {
    for (; *s; s  ) {
        if(!isalpha(*s)) continue;
        *s = (toupper(*s) - 'A'   k) % ('Z' - 'A'   1)  
             (isupper(*s) ? 'A' : 'a');
    }
    return s;
}

CodePudding user response：

The function as is does not make a sense.

It should not output anything. Its task is to encrypt the source string or to create a new encrypted string based on the source string and return it.

Also it is a bad idea to use magic numbers like 65 and 97.

And the name of the function rotate is confusing.

Apart from this either use the type char * for the parameter and the return type of the function or the typedef name string.

For example the function can look the following way

string encrypt( string plaintext, int k )
{
    for ( string p = plaintext; *p;   p )
    {
        if ( isupper( ( unsigned char )*p ) )
        {
            *p = 'A'   ( *p - 'A'   k ) % 26;
        }
        else if ( islower( ( unsigned char )*p ) )
        {
            *p = 'a'   ( *p - 'a'   k ) % 26;
        }
    }

    return plaintext;
}

Here is a demonstration program.

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

typedef char *string;

string encript( string plaintext, int k )
{
    for ( string p = plaintext; *p;   p )
    {
        if ( isupper( ( unsigned char )*p ) )
        {
            *p = 'A'   ( *p - 'A'   k ) % 26;
        }
        else if ( islower( ( unsigned char )*p ) )
        {
            *p = 'a'   ( *p - 'a'   k ) % 26;
        }
    }

    return plaintext;
}

int main( void )
{
    char plaintext[] = "Hello, World!";

    puts( plaintext );
    puts( encript( plaintext, 4 ) );
    puts( encript( plaintext, 22 ) );
}

The program output is

Hello, World!
Lipps, Asvph!
Hello, World!

As your function initially was named as rotate and the value of the parameter k can be negative because the parameter has the signed type int then it seems that each alpha character after encryption must be again an alpha character. If so then the function can be defined the following way as shown in the demonstration program below.

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

typedef char *string;

string encript( string plaintext, int k )
{
    for ( string p = plaintext; *p;   p )
    {
        if ( isupper( ( unsigned char )*p ) )
        {
            *p = 'A'   ( *p - 'A'   k ) % 26;
            if ( *p < 'A' ) *p = 'Z' - ( 'A' - *p  - 1 );
        }
        else if ( islower( ( unsigned char )*p ) )
        {
            *p = 'a'   ( *p - 'a'   k ) % 26;
            if ( *p < 'a' ) *p = 'z' - ( 'a' - *p  - 1 );
        }
    }

    return plaintext;
}

int main( void )
{
    char plaintext[] = "Hello, World!";

    puts( plaintext );
    puts( encript( plaintext, 4 ) );
    puts( encript( plaintext, -4 ) );
}

The program output will be the same as shown above

Hello, World!
Lipps, Asvph!
Hello, World!