Can I avoid all this multiples try/catch

Question

I have a vector of many boost::any In this vector I need to perform some operations on std::vector and on the elements of type IContainer

class   IContainer
{
public:
  virtual ~IContainer(){}
  virtual const boost::any operator[](std::string) const = 0;
};

class   AContainer : public IContainer
{
  std::vector<int>      vect_;
  std::string name_;
public:
  AContainer() : vect_({0, 1, 2, 3, 4, 5}), name_("AContainer") {}
  virtual const boost::any operator[](std::string key) const
  {
    if (key == "str")
      return (name_);
    if (key == "vect")
      return (vect_);
    return nullptr;
  }
};

So I have done the following function (imo quite ugly) but who works correctly

m is const std::vector<boost::any>&

for (const auto & elem : m)
    {
      try
        {
          std::vector<int> v = boost::any_cast<std::vector<int>>(elem);
          display(v);
        }
      catch(boost::bad_any_cast){}
      try
        {
          std::vector<IContainer*> v = boost::any_cast<std::vector<IContainer*>>(elem);
          display(v);
        }
      catch(boost::bad_any_cast){}
      try
        {
          AContainer v(boost::any_cast<AContainer>(elem));
          try
            {
              display(boost::any_cast<const std::vector<int>>(v["vect"]));
            }
          catch (boost::bad_any_cast){}
          try
            {
              std::cout << boost::any_cast<const std::string>(v["str"]) << std::endl;
            }
          catch (boost::bad_any_cast){}
          try
            {
              display(boost::any_cast<std::vector<int> >(v));
            }
          catch (boost::bad_any_cast) {}
        }
      catch(boost::bad_any_cast){}
    }

I have tried to add many "try{}try{}catch{}" but it's not working

Do you have any solutions better than what I have done

Edit

I have tried the solutions of James Kanze, user1131467 and Praetorian

So the 3 are working nicely, but when I have calculate the time of execution, the answer of user1131467 is a bit faster than the other. I must now find a solution to store each types in a map to avoid all this if/else

I will also take a look at boost::variant

Are you sure that `boost::any` is the right tool for the job? Have you considered different approaches like `boost::variant`? — David Rodríguez - dribeas, Jul 12 '13 at 15:10

Andrew Tomazos · Accepted Answer · 2013-07-12T15:27:28.070

7

Using the pointer-form of any_cast is much cleaner, as it uses the nullability of pointers:

for (const auto & elem : m)
    if (T1* p = any_cast<T1>(&elem))
    {
         do stuff with *p;
    }
    else if (T2* p = any_cast<T2>(&elem))
    {
         do stuff with *p;
    }
    else if (...)
    {
         ...
    }

This also has the advantage of doing the cast once per case.

edited Jul 12 '13 at 15:27

answered Jul 12 '13 at 14:49

Andrew Tomazos

66,139
40
186
319

`boost::any_cast` will return a null pointer, rather than throw, if the cast fails. You can also use `if ( elem.type() == typeid(TargetType) )` if there's no polymorphism. But while defining the pointers in the condition might be legal, it's very bad programming practice: it's better to do the `if`, then *if you need the pointer* define it within the controlled block. – James Kanze Jul 12 '13 at 14:54
@JamesKanze: Why do you think it is bad programming practice? – Andrew Tomazos Jul 12 '13 at 14:55
Because it leads to unreadable code. And `if` statement controls flow. It doesn't change state or introduce additional variables. (Having said that: this is a limit case where it might be justified. And your solution is certainly better than all those which try to play tricks with `try`/`catch`.) – James Kanze Jul 12 '13 at 14:58
@JamesKanze: A `for` statement controls flow as well, and also permits a declaration. You wouldn't argue that `for (int i = 0; i < n; i++)` is bad programming practice. – Andrew Tomazos Jul 12 '13 at 15:04
A `for` statement _always_ defines a variable, so you know that the definition is there. Defining more than one variable in a `for`, or defining the variable in the condition, wouldn't be the best practice. (But as usual, there are exceptions. This might be one, although to tell the truth, I probably wouldn't write it as a sequence of `if ... else if ...` anyway. More likely a `map`, looked up using `elem.type()`. – James Kanze Jul 12 '13 at 16:38
@JamesKanze: _"A for statement always defines a variable"_: The following for statement does not declare a variable `for (i = 0; i < n; i++) {...}`. Your statements about best practice are personal opinions given without sound rationale. – Andrew Tomazos Jul 12 '13 at 19:55
I probably miss-worded it: a `for` statement usually defines a variable. It doesn't have to, but part of the reason for using `for` instead of `while` is that it can. And code readability isn't purely subjective: statements which do more than one thing do make code harder to read and understand. – James Kanze Jul 13 '13 at 17:29

score 5 · Answer 2 · answered Jul 12 '13 at 14:50

5

You can create a function along the lines of:

template <typename T>
bool 
isInstanceOf( boost::any const& object )
{
    return boost::any_cast<T>( &object ) != nullptr;
}

and use it, with if's to check:

if ( isInstanceOf<std::vector<int>>( elem ) ) {
    display( boost::any_cast<std::vector<int>>( elem ) );
} else if ( isInstanceOf<std::vector<IContainer*>>( elem) ) {
    display( boost::any_cast<std::vector<IContainer*>>( elem) );
} 
//  ...

answered Jul 12 '13 at 14:50

James Kanze

150,581
18
184
329

Thanks for your answer, I have chosen the other one since I don't have to call 2 times the any_cast – Alexis Jul 12 '13 at 15:02
@Alexis: You shouldn't blindly pick what you accepted. The approach in this answer is much better at least in the sense that it won't cause a huge amount of exceptions in your system (which might be expensive). – David Rodríguez - dribeas Jul 12 '13 at 15:12
1

My answer is superior to both as (a) it doesn't use exceptions; (b) requires only calling `any_cast` once (not twice) per case; and (c) doesn't require defining new functions. – Andrew Tomazos Jul 12 '13 at 15:25
@user1131467 On the other hand, your answer results in a lot of extra pointer variables floating around. If you give them different names, there is a risk of using the wrong one because of a typo, and if you give them the same name, there is the risk of warnings from the compiler because of masking, or problems with the debugger for the same reason. (On the other hand, my solution requires repeating the type name twice, which isn't ideal either. In the end, an `std::map` indexed by `elem.type()` is probably a better solution.) – James Kanze Jul 12 '13 at 16:47
_"if you give them the same name, there is the risk of warnings from the compiler because of masking"_: hiding a variable from an outer scope in an inner scope is standard and well-formed, and does not produce a warning or problems with the debugger. If you were really paranoid about that you can always terminate each case with a jump statement, and then use if rather then else-if. I agree that if there are lots of cases an `unordered_map` is more performant than all solutions so far discussed. – Andrew Tomazos Jul 12 '13 at 19:53
@user1131467 It's standard and well formed, but generally considered bad programming practice: some compilers do warn when it occurs, and I seem to remember having problems with a debugger when I accidentally did it. And there is, at least until the profiler says otherwise, no argument with regards to performance: code with a single access to a map is probable easier to read and to understand than a sequence of a large number of `if ... else if...`. (More generally, a `switch` or a sequence of `if .. else if` is probably to be avoided except for values from user input.) – James Kanze Jul 13 '13 at 17:26

score 2 · Answer 3 · answered Jul 12 '13 at 14:53

2

You could write your own wrapper around any_cast that swallows exceptions.

template<typename T>
bool nothrow_any_cast( boost::any& source, T& out )
{
  try {
    out = boost::any_cast<T>( source );
  } catch ( boost::bad_any_cast const& ) {
    return false;
  }
  return true;
}

And then use it as

std::vector<int> vect;
std::string str;

if( nothrow_any_cast(v["vect"], vect ) ) {
  // succeeded
} else if( nothrow_any_cast(v["str"], str ) ) {
  // succeeded
} ...

However, if you do this, you're default constructing all the types, and then assigning them; so even if it looks a little cleaner, it's debatable whether it is any better than what you already have.

answered Jul 12 '13 at 14:53

Praetorian

106,671
19
240
328

Thanks I was looking for something like that. – Alexis Jul 12 '13 at 15:01
1

That is still horrible, the horror being just hidden under the covers... It also requires the definition of the variables in the enclosing scope, and still causes a huge amount of exceptions... – David Rodríguez - dribeas Jul 12 '13 at 15:08
@DavidRodríguez-dribeas I agree, James' answer is better than mine (I wasn't aware of the pointer overload for `any_cast`) – Praetorian Jul 12 '13 at 15:13

score 0 · Answer 4 · answered Jul 12 '13 at 14:38

0

You can put all of the statements in one try block:

try {
    // do all your stuff
}
catch (boost::bad_any_cast) {}

answered Jul 12 '13 at 14:38

sedavidw

11,116
13
61
95

yes but for exemple if it's a vector I must perform one operation and then stop, not go at the next line. and if I have a vector on the second line it's never going to reach it because the exception will be thrown before – Alexis Jul 12 '13 at 14:40
If you want something like that you might be able to do some exception handling with your catch statement and re-direct based upon the error thrown. Otherwise I think you're stuck with what you've got – sedavidw Jul 12 '13 at 14:43
It's what I was scared of, just asking to see if people have other ideas – Alexis Jul 12 '13 at 14:45
One thing to do if you REALLY WANTED on try block (though I think this is uglier) is to set unique booleans to true after each successfully completed line. Then based in your catch block you can use a goto to go to the next line after the line that threw an exception. Personally I think what you have now is cleaner – sedavidw Jul 12 '13 at 14:51
@sedavidw There's no need for _any_ `try` blocks in his code. – James Kanze Jul 12 '13 at 14:55

fiscblog · Answer 5 · 2013-07-12T14:45:36.527

0

You missed to add any instruction when entering your catch(...){} block. So when an exception is thrown you do not handle it at all.

An appropriate handling usually includes to trace the level of error and try to resolve the state of error (if possible).

Since in every branch you do catch the same exception you can aggregate all of them to one block:

try{ ... }
catch(boost::bad_any_cast) { ...}

edited Jul 12 '13 at 14:45

answered Jul 12 '13 at 14:39

fiscblog

694
1
12
27

The following code is working I don't want to perform any thing, just go the next try/catch block I would have loved to have some if/elseif/else with dynamic_cast but it's not working with boost::any – Alexis Jul 12 '13 at 14:42

Tiemo Jung · Answer 6 · 2013-07-12T16:42:39.397

Why not use the pointer alternative of boost::any_cast, it does not throw anything, it returns nullptr if the requested type does not match with the store type. And if you get a pointer back, you can just write an overload for display, that takes any pointer, checks it for being null and calls the actual display function if the pointer is not null.

template<typename T>
void display(T* p) { if ( p ) display(*p); }

With the template above, only display is called for the correct cast.

for ( const auto& elem : m ) {
  display(boost::any_cast<int>(&elem));
  display(boost::any_cast<my_type>(&elem));
  ....
}

score 0 · Answer 7 · answered Aug 21 '20 at 23:32

0

None of the answeres go the simple way that is now in the standard.

If you use "std::any" then you can just use the "type()" function to get the typeid of the containing item.

answered Aug 21 '20 at 23:32

Lothar

12,537
6
72
121

Can I avoid all this multiples try/catch

7 Answers7