Why does a control path not execute in this function?
Why does a control path not execute in this function?
In VS I am told this function "does not return a value in all control paths." A bot told me specifically the issue is with this line: else if (letter + key <= 90). It said that if the outcome results in letter + key equally exactly 90 then a value is not returned, but I thought that was covered where '<=' means 'less than or equals.'
char rotate(char letter, int key) { if (isalpha(letter) == true) { if (letter + key > 90) { int overage = letter + key - 90; letter = 64 + overage; while (letter > 90) { overage = letter - 90; letter += overage; } return letter; } else if (letter + key <= 90) { letter += key; return letter; } } else if (isalpha(letter) == false) { return letter; }
you are missing a returning
else
block.Ah I see. I had a bad habit of using else if statements instead of else statements because I thought else if could be better in seeing the condition it's testing for so it was clearer. I get the logic is actually different now.
I'm a fan of early returns.
So
I find it lets me mentally walk through all the paths more easily.
And if something gets too complex for this, then I need to break it down into further functions
It's not at all necessary, but I find it makes much easier to read code if you instead only use if statements and just return early when you're in a function. For example, you could check
isalpha(letter) == true
is true then checkletter + key <= 90
do theletter += key; return letter;
then since letter + key must be 90 if it didn't already return a value, then you can do the while statement and return letter without needing an if statement at all. Then theisalpha(letter) == false
is also unncessary, just return letter at the end.Like this:
I second the early return suggestions, but the other thing you should be aware of is that
isalpha()
is being evaluated twice unnecessarily. If its cheap and not call frequently, not a real big problem, but it is a waste of cycles. If you want to document the else, you could try:Also, if
isAlpha
was something that could change between evaluation, such asisTuesday
, you are at risk of the first call returning false, and then the second call returning true, which would skip both cases.you are checking the result of a method that returns a boolean (true/false) there is no reason to check for
true
,else
,FILE_NOT_FOUND
. you can also forgo equating its return value to something to get a boolean value required for the if statement since its already a boolean value.e.g.
https://en.wikipedia.org/wiki/Defensive_programming might be an interesting read.