Refactoring: Naming conditional statements
This is the first in a series of small tips that you can apply to make your code more readable, maintainable and generally just smell better. This one deals with conditional logic and how it is often unnecessarily hard to read. The problem is that many people, myself included, tend to pile a lot of different checks into the conditional statement, without regards for readability. I guess splitting up the statement into several lines of code triggers some base programmer instinct we all have for keeping our code tight and clean.
Regardless of why, I've found it very useful to break out the boolean expression into one or more variables that explicitly declare the semantic meaning of the expression. When several checks are combined that are not directly related, it can even be better to break it into several variables.
- // Bad code
- if ((user.HasRole("seller") || user.HasRole("manager")) && order.IsHandled && order.HandleDate < DateTime.Now)
- {
- // snip
- }
- // Better
- var orderIsProcessedAndCanBeSeenByUser = (user.HasRole("seller") || user.HasRole("manager")) && order.IsHandled && order.HandleDate < DateTime.Now
- if (orderIsHandledAndCanBeSeenByUser)
- {
- // snip
- }
- // Best
- var userCanSeeOrder = user.HasRole("seller") || user.HasRole("manager")) ;
- var orderIsProcessed = order.IsHandled && order.HandleDate < DateTime.Now;
- if (userCanSeeOrder && orderIsProcessed)
- {
- // snip
- }
There is one caveat with using the multi-variable approach, as you circumvent the short-circuiting mechanism of C#. If the second part of the check performs an expensive operation such as a database call, this approach will force it to be evaluated every time. If you're insistent, you can always wrap secondary expressions in a lambda function, but that will look a bit odd, I should think.
Comments