A developer sent a review for a method that was displaying a tooltip. He was fixing a simple “null reference” exception, but the logic for displaying the tooltip seemed weird:
If the text is greater than 100 characters, we don’t show a tooltip. If it is less than 100 then we do.
So I suggest that we always show the tooltip, but if it is greater than 100 characters, then we truncate to 97 characters plus some ellipses “…” to show the user that some text is missing.
At first, the developer responds that he was already truncating it. So I explain again that it doesn’t show at all in the case the text is greater than 100. He agrees to fix it. Later, he asked me to review the code again.
Now he is showing the tooltip if it is greater than 100 characters, but he is showing 100 plus the 3 ellipses to make 103 characters in total; which is over his defined limit. It’s a bit weird but not a huge problem. Additionally, he now no longer shows a tooltip when it is less than 100 characters, so he has just reversed the problem!
So I point out both the problems and he agrees to fix them. He asks me to review the code once more. Despite fixing these issues, he has now reintroduced the original problem that he was fixing! Now it will crash again with a “null reference exception”!
So I say
“Doesn’t this reintroduce the bug you were originally fixing?”.
It’s a rhetorical question of course, I know full-well that it does; but it’s more polite than an expletive-ridden comment questioning his intelligence.
“No, it doesn’t bring back the original bug back in – I have verified that.”
I ask him if “he is sure about that?”. I don’t even need to test it, I can see that it is 100% a problem just by reading the code. I’d expect a Junior to easily spot that without testing it. Not sure how he could miss it if he had done some proper testing.
“As per my basic testing it didn’t crash but I can do some more testing and let you know.”
1 hour goes by. He then replied:
“Just now I saw the mistake from my end”
(-‸ლ) 🤦