Let's look at your problem from another point of view. When designing a body of code, I try to apply the following principle:
- Make it correct.
- Make it clear.
- Make it concise.
- Make it fast.
... in that order.
All of these, to one extent or another, are subjective. However, reasonable people tend to find common ground - and there's often more agreement about their opposites. But that aside...
The first priority here is making sure that your code will work correctly. Clearly, there are multiple implementation that achieve this - but I would also add that it's important that it be easy to demonstrate that the implementation is correct. One way of achieving this is to make the code read more like the spec (more on this later).
The second priority is to make sure that in the future, when a developer (including the original author!) looks at this code they'll be able to understand what it's doing right away. The more sophisticated (read: fancy) the implementation, the harder it is for a developer to immediately understand what the code is doing.
The third priority - short, concise code, is one that partially stands in opposition to the first two. A desire to make the code more concise, may lead you to use more complex constructs than are actually necessary to solve the problem. While it's important to keep the code short, we shouldn't do this by making it unintelligibly dense.
The last priority - performance - only matters when it matters. By this, I mean that you shouldn't complicate the implementation from the point of view of performance unless you've performed profiling and identified it as bottleneck in your system.
So now that we've looked at the principles that should drive our decisions, let's apply them to the problem at hand. You've provided a very clear specification of how the code is supposed to behave. Let's try to adhere to them:
void YourMethod( int oldValue, int newValue )
{
bool oldValueNonZero = oldValue != 0;
bool newValueNonZero = newValue != 0;
if( oldValueNonZero ) { X(); }
if( newValueNonZero ) { Y(); }
if( oldValueNonZero && newValueNonZero ) { Z(); }
}
So why do I like this particular implementation. Let's break it down.
First, note that I've chosen to create temporary boolean to capture the result of testing the old/new value for whether they are nonzero. By capturing these values, I avoid performing the calculation more than once, and I also make the code more readable (see below).
Second, by choosing the descriptive names oldValueNonZero
and newValueNonZero
I'm making the implementation clearly indicate my expectations. This both improves the readability of the code and clearly conveys my intent to future developers who have to read it.
Third, note that the body of the if()
test is wrapped in {
and }
brackets - this helps reduce that chance that future changes to the implementation will break the behavior - by accidentally including a new case, for instance. Using single-line ifs
is a recipe for future problems.
Finally, I don't try to short-circuit the comparison and exit the function early. If performance were extremely important, an early exit may be useful. But otherwise, it makes it easier to understand the behavior of a method if there's only one exit point(1).
Does this code do what the spec says? I believe so.
Is it easy to read and understand. At least to my eyes, I would say yes.
Is this code the most compact or sophisticated way of short-circuiting the logic? Almost certainly not ... but it's other qualities more than make up for that, in my opinion.
Whether you like this particular structure of code or not is, to some extent, a matter of taste and style. But I hope that the principles I've laid out about how I chose to organize it may help you make such decisions in the future.
You've indicated that you sometimes run into similar "logic-grid" problems, but ones where the number of cases are more numerous. These kinds of problems can become complicated for two separate reasons:
- The number of values that the parameters can take on increase - they can take on the general form
MxN
.
- The number of dimensions increase - in other words, there are more variables to include in the rules:
MxNxOxP...xZ
.
One generalized solution to the problem (as another response indicates), is to encode the solution as multidimensional matrix - and define a set of actions for each case. However, it's entirely possible for the rules to overlap - and it's probably desirable to collapse equivalent cases together, for simplicity.
My response to dealing with the general case is ... that it depends. If the conditions can be reduced to some very small number of cases, than imperative if/else logic may still be the best way to solve the problem. If the number of conditions are very large, than it may make sense to use a declarative approach, in which you use some kind of lookup table or matrix to encode the cases.
1> - One common exception to the principle of only having a single exit point from a method is for preconditions. It's cleaner to avoid nesting and inverted logic by first checking all/any preconditions, and failing (exiting) the method if they are violated.