How can I efficiently write a complex if or switch condition that depends on a combination of two (or more) variables?

Question

In my current case, I've got two variables. Let's call them action and status for ease of use. They can have any value between 1 and 9. Depending on business logic, I've to either call subActionA() or subActionB(). I'm trying to write this in the most readable way, and so far I'm not happy with the results. What I've effectively gotten down to is the following

if(action==1){
    if(status==1){
    }else if(status==2{
    }else...
}else if(action==2){
    if(status==1){
    }else if(status==2{
    }else...
}else....

This solution, while it works, doesn't seem particularly clean. Is there a better way to represent this type of decision tree? The only thing that comes to mind for me is a nested switch, which would be as bad as this nested if.

How would this be extended for more variables, or if there were more than two subactions?

We're basically dealing with handling medical prescriptions. The actual 'action's are 'update','add','duplicate','sign','reject' etc. The 'status' is one of 9 statuses of the prescription as defined by the business. Based on the status and the action, we either edit the current prescription or create a new one.

EDIT 1: While How to tackle a 'branched' arrow head anti-pattern might seem similar, I think it's only superficially so. That question deals with branching based of sub conditions that only occur under some combination of earlier conditions, with each set variable being true or false. My question deals with writing a clean solution for all possible combinations of a set of variables that can have more that the fixed true and false values. Additionally, the linked question doesn't mention if the result of this conditional checking returns a different result for each set of conditions or, like mine, calls one of two functions based on the combination.

EDIT 2: The values for action and status are not actually integers. I've just represented them as so for ease of understanding. There's no mathematical relation between the values for both variables and which subaction gets called.

You could `switch()` on the value of `10 * action + status`... — Kilian Foth, Feb 26 '19 at 13:55
You _could_ do this by subtype polymorphism, but there's absolutely no way to tell what's the best way from your question. That being said, from your code, it does look like there's a design issue going on. — Vincent Savard, Feb 26 '19 at 14:08
Setting aside that there may be design problems here, this is not unreadable *per se*; it it's really only unreadable when you have a a lot of code within the bodies of the conditionals. If that's *not* the case, maybe you should just leave it, but if it is, switch statements and other clever tricks won't really help, so start tackling that - refactor, move those blocks into separate functions, look for similarities/duplication, remove those - and then maybe you'll see patterns emerge, maybe you'll extract stuff to a separate class, etc. — Filip Milovanović, Feb 26 '19 at 14:59
@VincentSavard, we're basically dealing with handling medical prescriptions. The actual 'action's are 'update','add','duplicate','sign','reject' etc. The 'status' is one of 9 statuses of the prescription as defined by the business. Based on the status and the action, we either edit the current prescription or create a new one. — Yash Capoor, Mar 01 '19 at 10:59
Having read the other questions, I disagree with the "duplicate" determination. Though close, the others deal with slightly more complex logic than the issue presented here. This question only depends on discrete input variables while the conditions on the other questions are a bit more general, which is probably why the other questions' answers don't offer the table-driven solution that would make so much sense in this case. — Adrian McCarthy, Mar 01 '19 at 19:05

score 2 · Answer 1 · answered Feb 26 '19 at 13:59

2

Depending on the language you use, you can switch on composite types. If you can't do that, you might be able to map your composite onto a primitive, e.g. 10 * action + status. You'd have something like

switch (ActionStatus(action, status)) 
{
    case ActionStatus(1, 1):
    case ActionStatus(1, 3):
    case ActionStatus(3, 1): { doThing1(); break; }

    case ActionStatus(1, 2):
    // ... more cases
    case ActionStatus(3, 4): { doThing2(); break; }

    // ... more actions
}

This will get unwieldy if there are large ranges for certain parameters.

answered Feb 26 '19 at 13:59

Caleth

10,519
2
23
35

Sorry for the confusion, but the values for `action` and `status` are not actually integers. I've just represented them as so for ease of understanding. There's no mathematical relation between the values for both variables and which `subaction` gets called. – Yash Capoor Mar 01 '19 at 10:51
@YashCapoor but can you assign each `action`, and each `status` a unique number? – Caleth Mar 01 '19 at 11:09

score 2 · Answer 2 · answered Feb 27 '19 at 16:31

Hopefully the integer values correspond to some meaningful names, like an action of "create" and a status of "complete". Your code would be clearer if it used these values (either as Strings or constants) instead of magic integers. The "divide and conquer" and do something like:

switch(action) {
  case CREATE: handleCreate(status); break;
  case DELETE: handleDelete(status); break;
  ...
  case HALT_AND_CATCH_FIRE: handleHACF(status); break;
}

Within the 10 different handleXXX() functions, you'd have another switch statement that does the actual work.

A bit tedious but clear.

They do have string constants, but I didn't want to add that to the question. We considered this solution, and it would add a lot of complexity as it would divide this one bit of business logic into multiple functions, adding length. I was hoping there would be a clean yet concise way of writing this. — Yash Capoor, Mar 01 '19 at 10:54

Adrian McCarthy · Answer 3 · 2019-03-01T18:59:32.530

Given that both inputs seem to be discrete and bounded, I would seriously consider using a two-dimensional table (or table of tables) of decisions indexed by action and status. This greatly simplifies the logic. You can then construct a small, simple table for unit-testing the logic regardless of the actual rules.

enum { UPDATE, ADD, DUPLICATE, kActions } Action;
enum { EXPERIMENTAL, APPROVED, kStatuses } Status;
enum { ISSUE, MODIFY, REISSUE } Decision;

const Decision DecisionTable[kActions][kStatuses] = {
               /* EXPERIMENTAL    APPROVED */
/* UPDATE */   {  REISSUE,        MODIFY  },
/* ADD */      {  ISSUE,          ISSUE   },
/* MODIFY */   {  REISSUE,        REISSUE }
};

void Handle(Action action, Status status) {
   switch (Decision[action][status]) {
       case ISSUE:    DoIssue(); break;
       case MODIFY:   DoModify(); break;
       case REISSUE:  DoReissue(); break;
       default:  DoError("Bad Decision in DecisionTable.");
   }
}

The table can be validated independently from the logic. If laid out well, it would be easy to see missed cases and interesting patterns. Non-programmers could inspect the table to check that it's right. You could even write an automated checker that looks for known-bad patterns in the table.

A table-based solution also makes it easier to update or correct the rules and have confidence that it doesn't introduce a stupid error in the code logic (like a missing break or an else matched to the wrong if).

If you need more input variables in the future, you could use more indirection (a table of tables of tables) as long as each new input is discrete and well bounded. If you need a new variable with more complex conditions (e.g., checking if the dosage is in some range), you might be able to devise a richer data representation that still separates the code logic from the decision data.

WTF. Oh, 10 more to go... – gnasher729 Mar 01 '19 at 19:57 — gnasher729, Mar 01 '19 at 19:57

score 0 · Answer 4 · answered Feb 27 '19 at 17:32

Personally I would use a combination of && and return to clarify the code. This reduces the mental tax of tracking else conditions, and makes it obvious which combination of which statuses results in which behavior.

void ChooseATask(int action, int status)
{
    if (action == 1 && status == 1)
    {
        DoSomething();
        return;
    }
    if (action == 1 && status == 2)
    {
        DoSomethingElse();
        return;
    }
    if (action == 2 && status == 1)
    {
        DoSomethingEvenStranger();
        return;
    }
    if (action == 2 && status == 2)
    {
        DoSomethingBeyondStrange();
        return;
    }
    throw new ArgumentException(string.Format("Invalid combination of action {1} and status {2}", action, status));
}

score 0 · Answer 5 · answered Mar 01 '19 at 13:22

It looks like the best situation to use if-else-if:

if (action == 1 && status == 1)
{
    DoSomething1();
}
else if (action == 1 && status == 2)
{
    DoSomething2();
}
else if (action == 2 && status == 1)
{
    DoSomething3();
}
else if (action == 2 && status == 2)
{
    DoSomething4();
}
else
{
    HandleImpossibleSituation();
}

It is clean and it also avoids multiple points of exit.

An alternative is:

if (action == 1)
{
    if (status == 1)
    {
    }
    else if (status == 2)
    {
    }
    else
    {
    }
}
else if (action == 3)
{
    if (status == 5)
    {
    }
    else if (status == 7)
    {
    }
    else
    {
    }
}
else
{
}

NOTE: it is always advisable to have a final else in an if-else-if construct, where you can handle the "impossible" situations.

How can I efficiently write a complex if or switch condition that depends on a combination of two (or more) variables?

5 Answers5