2

I am working on a code in which I shift each letter by one place, so (a) becomes (b) and (b) becomes (c) and so on. So far I managed to do that, but I am confronting a problem wrapping around the capital letter (Z) to (A). I can't seem to get the logic how to do that. Any help would be appreciated. Thanks a lot.

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

int main(void)
{
    //prompt the user to type in a text.
    string p = get_string("plaintext: ");
    
    //create a variable to refer to the length of the string.
    int n = strlen(p);
    
    
    for (int i = 0; i < n; i++)
    {
        //check if all chars (i) in the string (p) are alphabetical then increment i by 1.
        if (isalpha(p[i]))
        p[i]++;

        {
            //check if i has gone beyond the letter (z) and (Z).
            if ((p[i] > 'z') && (p[i] > 'Z'))
            {
                //if so then subtract 26 letter after z or Z to get back to a or A.
                p[i] = p[i] - 26;
            }

        }
        printf("%c", p[i]);
    }
    printf("\n");
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Bryce
  • 71
  • 8
  • 1
    Can you make it work by handling the lowercase and uppercase cases separately? At any rate, think carefully about your logic. `((p[i] > 'z') && (p[i] > 'Z'))` - one of those tests must simply be redundant, for the same reason that `(x > 10) && (x > 20)` is equivalent to just checking `x > 20`. – Karl Knechtel Jul 13 '20 at 12:26
  • Problem is with `if` condition which checks whether you have gone beyond 'z' or 'Z'. You would have to break the conditions based on case. For small case something similar to this: `if( smallCase(p[i]) & p[i]>'z') p[i]='a')` – svtag Jul 13 '20 at 12:29
  • `char U[27] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; for (int n = 20; n < 30; n++) putchar(U[n%26]);` – pmg Jul 13 '20 at 12:30

2 Answers2

2

an other way closer to the initial program is just to replace

if ((p[i] > 'z') && (p[i] > 'Z'))

by

if ((p[i] == 'z'+1) || (p[i] == 'Z'+1))

that avoid to duplicate almost all the code as this is the case in the other answer


And I think it is more readable to replace

p[i] = p[i] - 26;

by

p[i] -= 'z' - 'a' + 1;

The compiler replace 'z' - 'a' + 1 by its value and the expression explain the goal by itself


And to finish I think it is more clear to do

if (isalpha(p[i]))
{
  if ((p[i] == 'z') || (p[i] == 'Z'))
    p[i] -= 'z' - 'a';
  else
    p[i] += 1;
}

and that remove an increment for nothing

or to have only one line :

if (isalpha(p[i]))
  p[i] += ((p[i] == 'z') || (p[i] == 'Z')) ? 'a' - 'z' : 1;

but this is less readable probably


Out of that

printf("%c", p[i]);

is expensive and can be replaced by

putchar(p[i]);
bruno
  • 32,421
  • 7
  • 25
  • 37
  • 1
    @Bryce I iedited my answer to give a solution in one line, please read it – bruno Jul 13 '20 at 13:01
  • BTW, could you please explain more the logic of: p[i] -= 'z' - 'a' + 1; Thanks – Bryce Jul 13 '20 at 13:02
  • 1
    @Bryce `'z' - 'a' + 1` is how 26 comes from. if you have 'z'+1 to go back to 'a' you have to remove `'z' - 'a' + 1`. Of course it is the same in upper case and `'z' - 'a' + 1` == `'Z - 'A' + 1` because there is the same number of lower case and upper case ... fortunately ^^ – bruno Jul 13 '20 at 13:04
  • 1
    @Bryce `p[i] += ((p[i] == 'z') || (p[i] == 'Z')) ? 'a' - 'z' : 1;` is the contraction of all, that allows to add -25 or 1 depending on the case. 25 rather than 26 of course because this is not done after an increment like in your code – bruno Jul 13 '20 at 13:07
1

You need to separate your increments/checks into uppercase and lowercase blocks because, although the characters a ... z and A ... Z are most likely to be in sequence, they will be different sequences.

Something along these lines for your loop:

    for (int i = 0; i < n; i++) {
        //check if all chars (i) in the string (p) are alphabetical then increment i by 1.
        if (islower(p[i])) { // Lowercase letter check ...
            p[i]++;
            //check if i has gone beyond the letter (z).
            if (p[i] > 'z') {
                //if so then subtract 26 letter after z to get back to a.
                p[i] = p[i] - 26;
            }

        }
        else if (isupper(p[i])) { // Uppercase letter check ...
            p[i]++;
            //check if i has gone beyond the letter (Z).
            if (p[i] > 'Z') {
                //if so then subtract 26 letter after Z to get back to A.
                p[i] = p[i] - 26;
            }

        }
        printf("%c", p[i]);
    }

(Also, I'm assuming that, in your code, having the { after p[i]++; is a typo - otherwise, you're checking every character, even if it is not a letter. Your indentation suggests that this is not what you intend.)

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
  • you have just introduced to me another way to tackle this issue! You are my savior! Thank you so much :) – Bryce Jul 13 '20 at 12:57