caesar()
appears to simply return the position of characters b to z in the array in a remarkably over complex manner and omits a altogether! Moreover since alphabets
is not a null terminated string strlen()
is not a valid operation in any case. The "encryption" is done (incorrectly) by alphabets[j+key]
in main()
making caesar()
particularly poorly named - since that is not what it does at all.
The following function will return the the cipher of any character in alphabet
, and leave any other character unmodified:
char caesar( char x, int key )
{
const char alphabet[] = {'a','b','c','d','e','f','g','h',
'i','j','k','l','m','n','o','p',
'q','r','s','t','u','v','w','x',
'y','z'};
char cipher = x ;
for( int i = 0;
cipher == x && i < sizeof( alphabet );
i++ )
{
if( alphabet[i] == x )
{
cipher = alphabet[(i + key) % sizeof( alphabet )] ;
}
}
return cipher ;
}
It makes far more sense to pass key
to ceasar()
that to pass the constant alphabet
, and to do the encryption where is alphabet
is "known". Splitting the cipher steps between caesar()
and main()
as you had done is a poor design and lacks cohesion and has unnecessary coupling.
If the character x
appears in alphabet
it is modified by alphabet[(i + key) % sizeof( alphabet )] ;
. This adds the key
as you had, but also "wraps-around" (the %
modulo operation), so that for example for key = 1
, z
wraps around to a
rather then referencing a byte beyond the end of the alphabet
array as your code had. Critically, if it does not appear in alphabet
, it is unmodified - that is why cipher
is initialised with x
. The loop exits when cipher
is modified (cipher != x
), or when the end of alphabet
is reached.
Then in the iteration of plain_text
:
for (i = 0; i <= strlen(plain_text); i++ )
{
cipher_text[i] = caesar( plain_text[i], key ) ;
}
The <= strlen()
here is unusual, but here it ensures the nul terminator is copied to cipher_text
- it will not be modified by caesar()
.
Note that the the above solution only encrypts lower case text (as does your original code). There are other issues and ill-advised practices in your code, discussed in the comments but not perhaps directly relevant to your question, but using the above function, the following complete implementation addresses most of the issues:
#include <stdio.h>
#include <string.h>
char caesar( char x, int key ) ;
#define MAX_TEXT 128
int main()
{
char plain_text[MAX_TEXT] = "" ;
char cipher_text[MAX_TEXT] = "" ;
printf( "Plain text: " );
fgets( plain_text, MAX_TEXT, stdin ) ;
printf( "\nThe plain text is: %s\n", plain_text ) ;
printf( "Key: " ) ;
int key = 0 ;
scanf( "%d", &key );
for( size_t i = 0; i <= strlen( plain_text ); i++ )
{
cipher_text[i] = caesar( plain_text[i], key ) ;
}
printf( "\nThe cipher text is: %s\n", cipher_text ) ;
return 0 ;
}
Example:
Plain text: abc, xyz
The plain text is: abc, xyz
Key: 1
The cipher text is: bcd, yza
Modification to allow capital letters:
#include <ctype.h>
char caesar( char x, int key )
{
const char alphabet[] = {'a','b','c','d','e','f','g','h',
'i','j','k','l','m','n','o','p',
'q','r','s','t','u','v','w','x',
'y','z'};
char cipher = x ;
for( int i = 0;
cipher == x && i < sizeof( alphabet );
i++ )
{
if( alphabet[i] == tolower( x ) )
{
cipher = alphabet[(i + key) % sizeof( alphabet )] ;
if( isupper( x ) )
{
cipher = toupper( cipher ) ;
}
}
}
return cipher ;
}
Here the test alphabet[i] == tolower( x )
ignores the case, and then when a match is fount the application of cipher = toupper( cipher )
if x
is upper case, yields the upper case cipher.
Example output:
Plain text: aBc, XyZ 123
The plain text is: aBc, XyZ 123
Key: 1
The cipher text is: bCd, YzA 123
Note that instead of testing cipher = x
in the for loop, you could break
after cipher
is assigned in the loop - reducing the number of tests - but that arguably breaks structured programming "rules" - I would not criticise its use by others, but it is not my preference. You might also use isalpha(x)
in this case to skip the loop altogether, but that has implementation defined behaviours for accented characters for example, so if you were to extend the supported "alphabet", it might fail to work as intended.
If you only ever use the characters a to z in the alphabet as you have, then a further simplification is possible where the cipher can be determined arithmetically using character code values:
char caesar( char x, int key )
{
char cipher = tolower( x ) ;
if( isalpha( x ) )
{
cipher = ((cipher - 'a') + key) % ('z' - 'a' + 1) + 'a' ;
if( isupper( x ) )
{
cipher = toupper( cipher ) ;
}
}
return cipher ;
}
Strictly this assumes that the characters a to z are contiguous in the target character set, but that is universally true for any system you are likely to run this code on (i.e. not an IBM Z series mainframe or various antique mainframe/mini computers), and if not the alphabet
array solution remains valid. I point this out only because otherwise someone will place a comment about it as if it is really an issue.
To explain the expression: cipher = ((cipher - 'a') + key) % ('z' - 'a' + 1) + 'a'
:
(cipher - 'a')
- subtract the code for 'a' to get a value 0 to 25 for characters a
to z
.
... + key
- add the key "shift"
... % ('z' - 'a' + 1)
- this constant expression resolves to % 26
in practice, to "wrap-around".
... +
a- transform the range 0 to 25 back into character codes
ato
z`.