How do I create a clean cascading if structure in

Question

I'm using boost's regex library, and I'm finding that determining if a named match is found and then using that information is a little annoying. To detect a named match, I'd like to do this:

typedef boost::match_result<string::const_iterator> matches_t;
typedef matches_t::const_reference match_t;
boost::regex re("(?:(?<type1>aaaa)|(?<type2>bbbb)" /*...*/ "|(?<typeN>abcdefg)");
string str(SOME_STRING);
matches_t what;
boost::match_flag_type flags = boost::match_default;

if(regex_search(str.cbegin(), str.cend(), what, re, flags))
{
  if((match_t type1 = what["type1"]).matched)
  {
     // do stuff with type1
  }
  else if((match_t type2 = what["type2"]).matched)
  {
     // do stuff with type2
  }
  // ...
  else if((match_t typeN = what["typeN"]).matched)
  {
     // do stuff with typeN
  }
}

If that'd would work, that would be great. Scoping would be constrained to the if's body, memory can be used efficiently and it looks fairly clean. Sadly, it doesn't work as you cannot define a variable within a list. :(

This could have been a possibility:

if(regex_search(str.cbegin(), str.cend(), what, re, flags))
{
  match_t found = what["type1"];
  if(found.matched)
  {
     // do stuff with type1
  }
  else if((found = what["type2"]).matched)
  {
     // do stuff with type2
  }
  // ...
  else if((found = what["typeN"]).matched)
  {
     // do stuff with typeN
  }
}

But match_t is a const reference so it's not assignable. (tl;dr Also I don't know what the underlying type is and generally I don't really want to know as I'd prefer a more generic solution that I could use outside of this one example of regex. Even std::move() was used around what[...] it gets even more verbose and the documentation doesn't say that it uses move semantics for sub_match. All of this is moot of course due to the reason given in the first sentence of this paragraph.)

Another option is to do is this:

if(regex_search(str.cbegin(), str.cend(), what, re, flags))
{
  match_t type1 = what["type1"];
  if(type1.matched)
  {
     // do stuff with type1
  }
  else {
    match_t type2 = what["type2"];
    if(type2.matched)
    {
       // do stuff with type2
    }
    // ...
          else {
            match_t typeN = what["typeN"];
            if((match_t typeN = what["typeN"]).matched)
            {
               // do stuff with typeN
            }
          }
    // ...
    }
  }
}

Which I don't like due to deep nesting of braces.

Perhaps abusing a loop structure with breaks at the end of each if body like this:

if(regex_search(str.cbegin(), str.cend(), what, re, flags))
{
  do{
    {
      match_t type1 = what["type1"];
      if(type1.matched)
      {
         // do stuff with type1
         break;
      }
    }
    {
      match_t type2 = what["type2"];
      if(type2.matched)
      {
         // do stuff with type2
         break;
      }
    }
    // ...
    {
      match_t typeN = what["typeN"];
      if(typeN.matched)
      {
         // do stuff with typeN
         break;
      }
    }
  } while(0);
}

Which is better, but still not great. Using macros, much of the noise could be hidden from view. Like:

#define IF(declare, cond) do{{declare;if(cond){                
#define ELSE_IF(declare, cond) break;}}{declare; if(cond){     
#define ELSE break;}}{{                                        
#define END_IF break;}}}while(0);                              

if(regex_search(str.cbegin(), str.cend(), what, re, flags))
{
  IF(match_t type1 = what["type1"], type1.matched)
  {
     // do stuff with type1
  }
  ELSE_IF(match_t type2 = what["type2"], type2.matched)
  {
     // do stuff with type2
  }
    // ...
  ELSE_IF(match_t typeN = what["typeN"], typeN.matched)
  {
     // do stuff with typeN
  }
  END_IF
}

The braces are actually implied by the macros, but it makes for clearer reading by restating them.

One other option I can think of is to go into the boost::sub_match class and add a conversion function to convert that type to a bool who's return value would be that of the matched member. Then I could declare a match_t variable in the if expression and it would automagically be converted to a boolean value by the if. I'm not sure if I'm there yet and it's not generic.

Stylistically, are the ones I propose good or bad (only the last 3 actually work, so I'd probably confine comments to them).

Also, does anyone have any better suggestions? Please state why you think they are better.

Answer 1

You can write a wrap of match_t with an overloads of operator bool:

struct bmatch
{
    matches_t::const_reference ref;
    bmatch(matches_t::const_reference r)
    :ref(r)
    {}

    operator bool() const
    {
        return ref.matched;
    }
};

And then:

if (bmatch type1 = what["type1"])
{ //Use type2.ref
}
else if (bmatch type2 = what["type2"])
{ //Use type2.ref
}

For extra points you can also overload operator->:

    matches_t::const_reference operator->() const
    {
        return ref;
    }

Answer 2

You could do something like this (note this code is not tested against a compiler)

// create a map between match types and actions
std::map<std::string, std::function<match_t>> actions;
actions["type1"] = [&](match_t type) {...};

// iterate through the actions map and check each match type
for(auto kvp : actions)
{
   match_t type = what[kvp.first];
   if(type.matched)
   {
      // do stuff with this result
      kvp.second(type);
   }
}

Answer 3

It is generally recommended to avoid nested ifs - they make code harder to read. If there's nested if, it should probably replaced by a function call.

In your case, you need to use a loop.

your second example:

if(regex_search(str.cbegin(), str.cend(), what, re, flags))
{
  match_t found = what["type1"];
  if(found.matched)
  {
     // do stuff with type1
  }
  else if((found = what["type2"]).matched)
  {
     // do stuff with type2
  }
  // ...
  else if((found = what["typeN"]).matched)
  {
     // do stuff with typeN
  }
}

BEGS for a loop:

const char *types[] = {"type1", "type2", "typeN", 0};
for(const char **cur = types; *cur; cur++){
    found = what[*cur];
    if (found.matched){
         //initiate robot uprising
         break;
    }
}

All your other examples (IMO) are a bad coding style. I prefer to keep loops and ifs short. If it doesn't fit into 20 lines of code, then it better be doing something very complicated (which is not your case). If it doesn't do anything complicated, it needs to be restructured.