#include <cs50.h>
#include <math.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>
#include <stdio.h>
int k = 0;
int convUpperAsciiToAlphaIndex = 65;
int convLowerAsciiToAlphaIndex = 97;
int AlphaIndexWrap = 26;
int main(int argc, string argv[]) // get Key from user via Command-Line Argument
{
// printf("%i\n", i); test print to see value of i
if(argc != 2)
{
printf("Invalid command line argument (non-alphabetical character)\n");
return 1;
}
string r = argv[1];
int q = strlen(argv[1]);
for(int i = 0; i < q; i++) //iterate every char in string to see if isalpha
{
if(isalpha(r[i]) == 0)
{
printf("Invalid command line argument\n");
return 1;
}
}
printf("plaintext: ");
string plaintext = get_string();
if (plaintext != NULL)
{
int c = strlen(plaintext);
printf("ciphertext: ");
// printf("%i \n", c); test print strlen result
for(int h = 0; h < c ; h++)
{
char d = plaintext[h];
if(isalpha(d))
{
int m = (k % q); // Modulo wrap-around index tracking for Key
int v = (r[m]);
{
if((isupper(d) && isupper(v)))
{
// Converts Plaintext & Key from Upper ASCii to Alpha Index, ADDS the two resulting INTs (applies the Cipher), applies Modulo Wraparound for Plaintext Index
int x = (((d - convUpperAsciiToAlphaIndex) + (v - convUpperAsciiToAlphaIndex)) % AlphaIndexWrap);
// converts back to ASCii
int w = (x + convUpperAsciiToAlphaIndex);
printf("%c", toupper(w));
}
else if((isupper(d) && islower(v)))
{
int toupper(int v);
// Converts Upper Plaintext & Lower Key from ASCii to Alpha Index, ADDS the two resulting INTs (applies the Cipher), applies Modulo Wraparound for Plaintext Index
int x = (((d - convUpperAsciiToAlphaIndex) + (v - convLowerAsciiToAlphaIndex)) % AlphaIndexWrap);
// converts back to ASCii
int w = (x + convUpperAsciiToAlphaIndex);
printf("%c", toupper(w));
}
else if((islower(d) && isupper(v)))
{
int tolower(int v);
// Converts Lower Plaintext & Upper Key from ASCii to Alpha Index, ADDS the two resulting INTs (applies the Cipher), applies Modulo Wraparound for Plaintext Index
int x = (((d - convLowerAsciiToAlphaIndex) + (v - convUpperAsciiToAlphaIndex)) % AlphaIndexWrap); // d - 97 + 2...converts to alpha index from lower and adds Key
// converts back to ASCii
int w = (x + convUpperAsciiToAlphaIndex); // converts back to ASCII
printf("%c", tolower(w));
}
else if((islower(d) && islower(v)))
{
// Converts Plaintext & Key from Lower ASCii to Alpha Index, ADDS the two resulting INTs (applies the Cipher), applies Modulo Wraparound for Plaintext Index
int x = (((d - convLowerAsciiToAlphaIndex) + (v - convLowerAsciiToAlphaIndex)) % AlphaIndexWrap); // d - 97 + 2...converts to alpha index from lower and adds Key
// converts back to ASCii
int w = (x + convUpperAsciiToAlphaIndex);
printf("%c", tolower(w));
}
k++;
}
}
else
{
printf("%c", (plaintext[h])); // prints non-alpha chars
}
}
printf("\n");
}
return 0;
}
-
\$\begingroup\$ @user3629249, you've posted some good advice in comments; would you be willing to write them in an answer? I'll vote if you do. \$\endgroup\$– Toby SpeightMar 20, 2017 at 11:58
-
\$\begingroup\$ @user3629249 Thanks for the feedback. I'm working solely within the CS50IDE "Appliance" for the course's Problem Sets. The code does compile and run successfully within that environment. Thank you for reminding me to avoid the use of a "magic number", that slips my mind often. \$\endgroup\$– Brian Patrick HummelMar 20, 2017 at 15:36
-
\$\begingroup\$ I've replaced the originally posted code with a refined version, taking into account @user3629249's suggestions! \$\endgroup\$– Brian Patrick HummelMar 20, 2017 at 16:00
-
\$\begingroup\$ moved my comments to an answer. \$\endgroup\$– user3629249Mar 20, 2017 at 16:21
1 Answer
none of the functions, etc available in math.h
are being used in the posted code. It is a bad idea to include header files that are not being used.
NONE of the functions, etc in cs50.h are portable, Strongly suggest using the actual C language types and C library functions
when there is something wrong with the command line parameter list, the resulting error message should be output the stderr
, not stdout
and it should be a 'USAGE' message, similar to:
fprintf( stderr, "USAGE: %s <stringToEncode>\n", argv[0] );
the function: get_string()
is not part of the cs50.h header and is not defined in the posted code. Perhaps you meant: GetString()
. This shows that either you have not compiled the posted code or the posted code is not your actual code.
in general, it is highly more understandable if the code uses literals like 'a' or 'A' rather than the hardcoded values, And the 26 is a 'magic' number. magic numbers have no basis. Suggest using a enum
statement or #define
statement to give that 'magic' number a meaningful name then using the meaningful name throughout the code.
in the new code these lines:
int convUpperAsciiToAlphaIndex = 65;
int convLowerAsciiToAlphaIndex = 97;
int AlphaIndexWrap = 26;
just clutter the code and consume stack space.
Suggest replacing all occurrences of convUpperAsciiToAlphaIndex
with 'A'.
Suggest replacing all occurrences of convLowerAsciiToAlphaIndex
with 'a'.
Suggest replacing this line
int AlphaIndexWrap = 26;
with:
#define ALPHABET_SIZE 26
and replacing all occurrences of AlphaIndexWrap
with ALPHABET_SIZE
variable names should indicate content
or usage
(or better, both).
This code:
if(argc != 2)
{
printf("Invalid command line argument (non-alphabetical character)\n" );
}
is not true and does not convey to the user what the problem is and does not output this error message to stderr
. See my earlier comment about how to correct these code lines.
for ease of readability and understanding: separate code blocks (for, if, else, while, do...while, switch, case, default) via a single blank line.
this line:
printf("Invalid command line argument\n");
has problems:
- fails to output the error message to
stderr
- fails to give the user any reason as to why the command line argument is not correct.
After the code expends lots of effort to validate the command line argument, then it gets a completely new string from stdin
(I.E. the user) to actually encrypt. And the new string is not validated at all.