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; }
DmMacniel@feddit.de 11 months ago
you arent returning something when neither your
if
nor yourelse if
checks out. given thatisalpha
returns a boolean, you can forgo yourelse if (isalpha(letter) == false)
since you already check for one of the case (true) and the remainder is just one case (false) thuselse
will suffice.milon@lemm.ee 11 months ago
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.
towerful@programming.dev 11 months ago
I’m a fan of early returns.
So
if(isalpha(letter) == false) return letter if(letter whater) { Do thing Return letter } if(letter something else) { Do whatever Return letter } throw error(“unprocessable letter”)
If find it lets me mentally walk through all the paths.
And if something gets too complex for this, then I need to break it down into further functions
ShaunaTheDead@kbin.social 11 months ago
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:
`char rotate(char letter, int key)
{
if (isalpha(letter) {
if (letter + key <= 90)
{
letter += key;
return letter;
}
}
return letter;
}`
CameronDev@programming.dev 11 months ago
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.DmMacniel@feddit.de 11 months ago
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.
heeplr@feddit.de 11 months ago
en.m.wikipedia.org/wiki/Defensive_programming might be an interesting read.