Decisions 2 (if ... elseif ... elseif ... elseif ...)
Speaking of the difference between a choice and a decision, the number of choices you have to pick from can deeply influence the decisions you make about the code you write. Because the number of choices you have can sometimes drive the implementation of the decision of how to do something far from where you actually make the decision.
You could do it up-front, with a set of micro-decisions, but you better know what the options are and make all the right choices then. If you do it that way though you don’t have much flexibility, because by the time the decision starts to impact what happens you’re far from the decision and it’s often too late to adjust. A better choice is to push the decision up the stack and abstract away the implementation down the stack to where more (context and detail) is known and the impact is clearer.
Another way to look at it is to think about some simple non-branching code. It proceeds from statement to statement doing whatever it’s told. All the choices were made by the compiler a long time ago. At runtime there are no decisions to be made.
Now add a simple if statement. Everything runs along smoothly, but somewhere in the middle there’s a decision and something different happens. If there’s only one of them then it’s a special case and (almost) everyone knows about it and expects it.
Of course, as time goes on more special cases show up, and the simple if
turns into an if ... elseif ... elseif ...
.
They look clean. But pretty soon new requirements show up. Special case upon special case,
with sub-cases and caveats, to the point where you build a truth table, hope you get it right, then implement it.
Eventually no-one is sure what happens, or why. Look at this code I wrote a year ago. It got so bad I put in a
comment explaining what was supposed to happen because it wasn’t obvious to me, let alone someone else, from
reading the code.
var filteredEntries []userstate.StateEntry
initialTargetLen := len(targets)
for _, entry := range entries {
req := entry.Value.(authutils.GetCredentialsRequest)
// This is getting complicated, so breaking it down. We EXCLUDE a profile if ALL of these conditions are false
// 1) The user specified a list of profiles and the current profile is one of them
// 2) The user did not specify a list of profiles AND specified the --all flag OR the --aws flag
// 3) The user did not specify a list of profiles, the current profile is not an admin profile,
// and the user did NOT specify the --admin flag
// 4) The user did not specify a list of profiles, the current profile is an admin profile,
// and the user specified the --admin flag
if !((initialTargetLen > 0 && targets[entry.Key]) ||
(initialTargetLen == 0 && (refreshParams.Flags.All || refreshParams.Flags.Aws)) ||
(initialTargetLen == 0 && roleIsAdmin(req.AWSRole) == refreshParams.Flags.Admin)) {
continue
}
filteredEntries = append(filteredEntries, entry)
delete(targets, entry.Key)
}
Try adding a new parameter that interacts with that logic. Combinatorial explosion. And it’s possible (likely) that some of the flags are mutually exclusive. It needs to be fixed, and one day I’ll get to it. Probably the next time I need to add something. At that point I’ll figure out how to extract the logic in some flag/parameter abstract way.
The problem is that the decision (what the user wants to do) is made far away temporally from when the code needs to decide. And on a completely different timeline new requirements get added. And this code makes that hard.
Making that kind of change easy is an architectural decision. There are lots of ways to do it. Everything from changing the requirements to objects, abstractions, inheritance, and interfaces. And they should all be considered before a decision is made.