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

 int main(int argc, string argv[])
 {
      string k = argv[1];
      string s = GetString();
      int l = strlen(k);

      for(int i = 0, n = strlen(s); i < n; i++)
      {
          if(s[i] >= 65 && s[i] <= 90)
          {
              int i2 = ((s[i]-65) + (k[i%l]-97)) % 26;
              printf("%c", i2+65);
          } else if(s[i] >= 97 && s[i] <= 122)
          {
              int i2 = ((s[i]-97) + (k[i%l]-97)) % 26;
              printf("%c", i2+97);
          } else
          {
              printf("%c", s[i]);
          }
      }
      printf("\n");
      return 0;
 }

I have removed as many parts as I can in order to make the code more relevant to the question. Basically why does this code work when "s" does not have any space(" ") in it and doesn't when "s" consists of space(" ")?

As most of you may know the idea is the argument entered at argv[1] is the "keyword" for the cipher. User then inputs a "plain-text" to cipher (s). It works when I try with various words or sentences if it doesn't include any space, " ". I just don't understand the logic behind this. Why does the cycle break if s[i] is not one of the first two conditions - I would have thought that "else" condition would work.

I would really appreciate it if someone can shed some light on this - many thanks in advance!

ps: I know there are some extra libraries at the top and the user input at argv[1] is not verified via isalpha(). I just want to understand the cycle process better for now, I have those checks in another file ready.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
HakBon
  • 63
  • 1
  • 9
  • 5
    Suggest using `'a'` and `'z'` (and `'A'` and `'Z'`) in place of numbers. – Jonathan Leffler Jan 20 '16 at 21:05
  • How your `GetString` function works? And what do you mean by 'when "s" consists of space(" ")'? – nsilent22 Jan 20 '16 at 21:07
  • 2
    When you treat a non-alpha character, you skip a letter in the key because you're using `i` to step through both the key and the data to be encrypted. You need to use separate indexes. – Jonathan Leffler Jan 20 '16 at 21:07
  • @nsilent22: `GetString()` is a semi-standard function when the `` header is included. You can find the source online rather easily; it appears quite often here on SO. – Jonathan Leffler Jan 20 '16 at 21:08
  • General rule in programming: do not use magic numbers. – too honest for this site Jan 20 '16 at 21:08
  • @JonathanLeffler: Does it read string until the newline character (I don't have such cs50.h header)? – nsilent22 Jan 20 '16 at 21:15
  • The header says: `/** * Reads a line of text from standard input and returns it as a * string (char *), sans trailing newline character. (Ergo, if * user inputs only "\n", returns "" not NULL.) Returns NULL * upon error or no input whatsoever (i.e., just EOF). Leading * and trailing whitespace is not ignored. Stores string on heap * (via malloc); memory must be freed by caller to avoid leak. */ string GetString(void);` (only the comment has 6 lines of text and a top and bottom line, and the declaration appears on the 9th line of the quoted fragment). – Jonathan Leffler Jan 20 '16 at 21:17
  • See also [CS50](http://cs50.net/) and [`GetString()`](http://reference.cs50.net/cs50.h/GetString) – Jonathan Leffler Jan 20 '16 at 21:26
  • Your code works correctly for me, and it also looks correct so long as the *key* only contains lower-case letters. (Putting spaces in the key might break the code, but you said you only put spaces in the input string). I tested your code with `hello` as the key (supplied on commandline) and `The quick brown fox` as input (supplied during execution) and got the output `Alp ebmnv ivzhb jzi`. If you are still having trouble please update your question to include the exact values you used for key and string , and the exact output you got. – M.M Jan 21 '16 at 04:08
  • It might be wise to validate the key, e.g. `for (int i = 0; i < l; ++i) if ( ! islower(k[i]) ) { printf("Invalid key.\n"); return EXIT_FAILURE; }` – M.M Jan 21 '16 at 04:10
  • @JonathanLeffler you sir are a lifesaver! Your second comment made it clear and after few changes I was able to get the code working. I hadn't truly understood the fact that while running a "for" loop when can have more than a single variable iterate! Many thanks! – HakBon Jan 21 '16 at 23:30

1 Answers1

0

Here is code that implements the 'separate counters for string and key' comment that I made. It also uses the letter codes 'a' and 'A' (and avoids needing to use 'z' or 'Z') instead of using numbers. It does assume that you are dealing with a single-byte code set (not UTF-8 unless you're working in the ASCII range) where the lower-case and upper-case letters are each in a contiguous range (so it won't work reliably with EBCDIC, but will with most other code sets), and it also ignores accented characters. (It would have to do setlocale("") to get locale-specific interpretations of which characters are letters.)

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

int main(int argc, string argv[])
{
    if (argc != 2)
    {
        fprintf(stderr, "Usage: %s key\n", argv[0]);
        return 1;
    }

    string k = argv[1];
    int l = strlen(k);

    for (int i = 0; i < l; i++)
    {
        int c = k[i];
        if (!isalpha(c))
        {
            fprintf(stderr, "%s: non-alpha character %c in key string\n", argv[0], c);
            return 1;
        }
        k[i] = tolower(c);
    }

    printf("Enter a string to be encrypted:\n");
    string s = GetString();
    int n = strlen(s);

    for (int i = 0, j = 0; i < n; i++)
    {
        int c = (unsigned char)s[i];
        if (isupper(c))
            c = ((c - 'A') + (k[j++ % l] - 'a')) % 26 + 'A';
        else if (islower(c))
            c = ((c - 'a') + (k[j++ % l] - 'a')) % 26 + 'a';
        putchar(c);
    }
    putchar('\n');

    return 0;
}

Here is a sample run that demonstrates the weakness of using 'a' as one of the letters in the key for this Vigenere cipher:

./vc caesArandAbrAcaDabRa
Enter a string to be encrypted: 
It is reported that Caesar said "Veni, vidi, vici" when he conquered Britain.
Kt mk rvpbutfu tjaw Cbvscr wsiu "Vrqi, wzdk, vlcj" nhgn lw cfndxesvd Drltbzn.
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278