Thursday, July 14, 2011

MySQL, Enum, skip the if()

There are a number of different, and very valid patterns for handling objects of different types. This is not about that, this is about how to not mix a pattern.


A very, very common bit of code that is in MySQL (and can therefor be found in Drizzle):


if ((cached_result_type == DECIMAL_RESULT) or (cached_result_type == INT_RESULT))


{


do_something();


}


else


{


do_something_else();


}



DECIMAL_RESULT and INT_RESULT are each possible result types.


Are there more?


Why yes there are. In the above bit of code the original author thought about two cases, and assumed all other cases could just be lumped into the else.


I’ve fixed dozens of bugs over the last few years based on similar assumptions.


What assumptions? 


1) The no one would ever add another result type.


2) That no other bug fix might create a case where the else no longer held true.


3) That the else was ever correct in the first place.


Without changing the entire design, what would be better?


Use a switch and make a case for each enum. That way if a new enum is added anywhere in the code where logic is required based on the enum you will catch it when you compile (assuming you have your warning flags turned up in your compiler). 


Also? Skip “default”. Unless you are taking something off the wire/file/etc you can skip default because you aren’t going to end up with an invalid enum. If you are doing one of these actions?


Sanitize the data first, don’t just cast it.

No comments:

Post a Comment