Your
is_digit
function is fundamentally broken. Even though there is a loop in place to try to check all the characters in the input string it will only check the first character in the string for being a digit and then it returns immediately. See if you can figure out the problem here. For example, it would think the string "1FOO" is only digits, but it is not. See if you can figure this out yourself.You need to do something with the returned result from your call to
only_digits
.- Its signature specifies it returns a bool which is correct. However if it's returning a bool you should really return
true
orfalse
to be proper instead of0
and1
. - That aside, I would stick to having cohesive functions of singular responsibility. Let that function only tell you if there are only digits or not. Move the business logic ( printing ) back into main.
- In the main function what do you need to do if
only_digits
indicates that the key is not "only_digits" so that you exit the program? Currently you do not examine the result of the function so flow continues to the next line of code.
- Its signature specifies it returns a bool which is correct. However if it's returning a bool you should really return
Minor Issue
- You have an issue with the size you use for
cipher_textciphertext
; it is incorrect. You need to remember thatstrlen
function doesn't give you the size you need to store the string, it gives you the length of the string, you need room for a null terminator. You should make the sizetext_length + 1
instead and remember to set the null terminator atcipher_text[text_length]ciphertext[text_length]
either before or after your population ofcipher_textciphertext
. If you don't do this, you will run into undefined value issues and your code will behave inconsistently. That's bad. Here I am spelling out a fix for you. UPDATE to set the null terminator you just have to sayciphertext[text_length] = 0;
orciphertext[text_length] = '\0';
- You have an issue with the size you use for
Problem with your construction of
cipher_textciphertext
- Your
rotate
function, takes in achar
and rotation number as input. Because you pass the inputchar
by value it does not alterplain_textplaintext
. As a result in the next line when you saycipher_text[i]ciphertext[i] = plain_text[i]plaintext[i]
effectively nothing happens, it just copies over the original value ofplain_text[i]plaintext[i]
. The return of your call torotate
is not used and discarded, but it does contain the correctly rotated letter, what could you do to make use of it?
- Side Notes
- I would suggest not declaring
cipher_textciphertext
the way you have; yes it is valid in C99 but in general that is not C style. If you don't know array size at compile time from a constant you should generally use dynamic allocation. ( which means a call to malloc and free ) Yes C99 lets you do this but you should get used to that idiom. - No reason to create variable
j
, just usetext_length
- You should avoid using 'magic' numbers and make them proper constants instead. Namely in your rotate function, yes I know why you are using
65
and97
but you should make them constants. Or see if you can figure out a way to do this without having to know the values65
or97
directly. Even26
you should probably make a constant but that number you is reasonable for you to 'know' so to speak.
- I would suggest not declaring