Range Checking
-
Some production VB.Net code I happened across and fixed (variable names and such changed to protect the innocent):
If number > 10 Or number < 30 Then
allowed = 1
End IfYou should notice two things strange about that code. Also, that code to check a range was hardcoded right after code that looks up ranges in a database and performs the same check. Perhaps it was added by somebody without database permission? :doh:
-
Some production VB.Net code I happened across and fixed (variable names and such changed to protect the innocent):
If number > 10 Or number < 30 Then
allowed = 1
End IfYou should notice two things strange about that code. Also, that code to check a range was hardcoded right after code that looks up ranges in a database and performs the same check. Perhaps it was added by somebody without database permission? :doh:
It could have been written before the db contained the validation or without knowledge of the previous. (I know I'm making assumptions and such. It's late and I'm ready to go to bed.)
I wasn't, now I am, then I won't be anymore.
-
Some production VB.Net code I happened across and fixed (variable names and such changed to protect the innocent):
If number > 10 Or number < 30 Then
allowed = 1
End IfYou should notice two things strange about that code. Also, that code to check a range was hardcoded right after code that looks up ranges in a database and performs the same check. Perhaps it was added by somebody without database permission? :doh:
I'm currently ROFL-ing inside, guys!!! A true pearl. Notice, that ANY int suits the condition )))
-
I'm currently ROFL-ing inside, guys!!! A true pearl. Notice, that ANY int suits the condition )))
I must have really been tired last night not to catch that. I'm having the same laugh now. :doh:
I wasn't, now I am, then I won't be anymore.
-
Some production VB.Net code I happened across and fixed (variable names and such changed to protect the innocent):
If number > 10 Or number < 30 Then
allowed = 1
End IfYou should notice two things strange about that code. Also, that code to check a range was hardcoded right after code that looks up ranges in a database and performs the same check. Perhaps it was added by somebody without database permission? :doh:
With that kind of range checking, the coder is more suited to checking the range where the burgers are being cooked. :)
Chris Meech I am Canadian. [heard in a local bar] In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
-
It could have been written before the db contained the validation or without knowledge of the previous. (I know I'm making assumptions and such. It's late and I'm ready to go to bed.)
I wasn't, now I am, then I won't be anymore.
The fact that an integer is being set to 1 or 0 rather than using a bool is a pretty good indicator that some of this code was copied from the database. Here is what my best guess is as to how this code evolved: *Only single numbers were checked, so they were put in a database table. *Next, certain ranges needed to be checked, and they were hard coded in the SP that checked single numbers. *Somebody thought the SQL code looked ugly or needed to use the SP without ranges, so they moved that to the VB.Net. *Somebody thought the VB.Net looked ugly, so they added ranges to the database table. *At some point, there was some VB.Net copy/pasting so that hard coded logic stayed. Result: X|
-
I'm currently ROFL-ing inside, guys!!! A true pearl. Notice, that ANY int suits the condition )))
Indeed, that was the real bug I fixed and one of the two things that was odd about the code (excluding the fact that it's VB.Net rather than C#, which is obviously a flaw too). :)
-
With that kind of range checking, the coder is more suited to checking the range where the burgers are being cooked. :)
Chris Meech I am Canadian. [heard in a local bar] In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
Either that, or being brought to the range to be slaughtered. Which wouldn't be much different, because he'd end up at the burger place anyway. :)
-
The fact that an integer is being set to 1 or 0 rather than using a bool is a pretty good indicator that some of this code was copied from the database. Here is what my best guess is as to how this code evolved: *Only single numbers were checked, so they were put in a database table. *Next, certain ranges needed to be checked, and they were hard coded in the SP that checked single numbers. *Somebody thought the SQL code looked ugly or needed to use the SP without ranges, so they moved that to the VB.Net. *Somebody thought the VB.Net looked ugly, so they added ranges to the database table. *At some point, there was some VB.Net copy/pasting so that hard coded logic stayed. Result: X|
aspdotnetdev wrote:
integer is being set to 1 or 0 rather than using a bool is a pretty good indicator that some of this code was copied from the database
I would have agreed with you until I say some javascript web page logic a while back that was hand coded to work with 1s and 0s instead of booleans. Now I just think there are some pretty thick junior devs out there being allowed to write code that gets to prod without review.
I wasn't, now I am, then I won't be anymore.
-
Indeed, that was the real bug I fixed and one of the two things that was odd about the code (excluding the fact that it's VB.Net rather than C#, which is obviously a flaw too). :)
aspdotnetdev wrote:
VB.Net rather than C#
Very obviously...
I wasn't, now I am, then I won't be anymore.
-
Either that, or being brought to the range to be slaughtered. Which wouldn't be much different, because he'd end up at the burger place anyway. :)
Shhh!!! Don't give John any ideas!!!
I wasn't, now I am, then I won't be anymore.
-
I'm currently ROFL-ing inside, guys!!! A true pearl. Notice, that ANY int suits the condition )))
Heh heh.. messed up boolean conditions are the bane of so many.. I think boolean logic is one of the hardest things to read well, and is one reason why I unit test my code thoroughly. Misused AND and OR is so easy to miss on code inspection.. but good tests never lie.
-
aspdotnetdev wrote:
integer is being set to 1 or 0 rather than using a bool is a pretty good indicator that some of this code was copied from the database
I would have agreed with you until I say some javascript web page logic a while back that was hand coded to work with 1s and 0s instead of booleans. Now I just think there are some pretty thick junior devs out there being allowed to write code that gets to prod without review.
I wasn't, now I am, then I won't be anymore.
I have seen people use ints with 1s and 0s because they learned programming with a language that did not have a boolean type. I say that is what you get when you have someone whose degree is not in programming write the software :sigh: What is really scary is when folk with a real programming degree do the same thing! :omg:
Just because the code works, it doesn't mean that it is good code.