0

This might be a case for the switch-off rule explained in C++ coding standards and I am wondering if I am doing it correctly. I am wondering because I still have if-clauses in the switching function.

Class A never gets instantiated directly, it's always either B or C that get dynamically created and uniformly handled through a (shared) pointer to A. foo switches and selects the operation depending on whether it's an B or C.

class A {
public:
  virtual ~A(){}
};

class B : public A {};
class C : public A {};

typedef std::shared_ptr<A> Aptr;
typedef std::shared_ptr<B> Bptr;
typedef std::shared_ptr<C> Cptr;


template<class T>
std::shared_ptr<T> get(const Aptr& pA) {
  return std::dynamic_pointer_cast< T >( pA );
}

void foo( const Bptr& pB ) {
  std::cout << "operate on B\n";
}

void foo( const Cptr& pC ) {
  std::cout << "operate on C\n";
}

void foo( const Aptr& pA ) {
  if ( auto x = get<B>(pA) ) {
    foo(x);
    return;
  }
  if ( auto x = get<C>(pA) ) {
    foo(x);
    return;
  }
  assert(!"oops");
}


int main()
{
  Aptr pA( new C );

  foo( pA );
}

My question is whether void foo( const Aptr& pA ) can be implemented more elegantly. That could mean without if. Is throwing in get and catching in foo recommended in this situation?

ritter
  • 7,447
  • 7
  • 51
  • 84
  • Use a virtual function? – GManNickG Mar 02 '13 at 21:53
  • `B` and `C` have very different interface. Some are common and remain in `A`. So, i don't want to operate through virtuals. – ritter Mar 02 '13 at 21:55
  • `B` and `C` can still have different interfaces beside `foo()`. – Olaf Dietsche Mar 02 '13 at 21:59
  • 1
    You're doing a Bad Thing. You've written a function `foo` that accepts any `A`, but can't actually act on all instances of `A`, only those that are of type `B` or `C`. If those are the only classes derived from `A` then you've done a slightly different Bad Thing, which is to rely on knowing an exhaustive list of all classes in a hierarchy (and on nobody ever instantiating A directly). You shouldn't really *expect* a Bad Thing to be elegant. – Steve Jessop Mar 02 '13 at 22:05
  • My example was too much boiled down. Of course virtuals can do this and are more accurate here. Since I don't like editing a question so it has a different solution, I will probably post a new question. – ritter Mar 02 '13 at 22:10
  • @SteveJessop ok, makes sense. So, (pure) virtual function for `foo` is the solution to this example. – ritter Mar 02 '13 at 22:16
  • @Frank: if you're "allowed" to modify the classes `A`, `B`, `C` then yes, a virtual member function is probably the right thing to do. It's a common piece of C++ advice, though, that where possible you should add functionality as free functions, not member functions. This advice is somewhat in tension with usual OOP practice. By following it you *might* sometimes find yourself wanting to do something like this, but it's rarely worthwhile and you're creating trouble down the line if there's no "else" case that can handle any `A`. – Steve Jessop Mar 02 '13 at 22:21
  • For those interested, please find the related question here: http://stackoverflow.com/questions/15180890/avoid-switching-on-types-to-allow-constant-folding – ritter Mar 03 '13 at 00:05

2 Answers2

2

Unless you have good reasons for doing otherwise (and if you have them, your code does not show them), this seems to me like the typical use case for dynamic polymorphism achieved through a virtual function:

class A 
{
public:
    virtual ~A() {}
    virtual void foo() = 0;
};

class B : public A 
{
    virtual void foo() 
    {
        std::cout << "operate on B\n";
    }
};

class C : public A 
{
    virtual void foo() 
    {
        std::cout << "operate on B\n";
    }
};

Besides, in C++11 it is preferable to use std::make_shared<>() over the construction of a shared_ptr with a naked new allocation (again, unless you have good reasons to do otherwise):

int main()
{
    Aptr pA = std::make_shared<C>();
    pA->foo();
}

If you have reasons not to use virtual functions and prefer a different, non-intrusive kind of polymorphism, you may use Boost.Variant in combination with boost::static_visitor. This does not even require B and C to be related.

#include <boost/variant.hpp>
#include <memory>
#include <iostream>

class B /* ... */ {};
class C /* ... */ {};

// ...
typedef std::shared_ptr<B> Bptr;
typedef std::shared_ptr<C> Cptr;

struct foo_visitor : boost::static_visitor<void>
{
    void operator () (Bptr p)
    {
        std::cout << "operate on B\n";
    }

    void operator () (Cptr p)
    {
        std::cout << "operate on C\n";
    }
};

int main()
{
    boost::variant<Bptr, Cptr> ptr;
    ptr = std::make_shared<C>();

    foo_visitor v;
    ptr.apply_visitor(v);
}

This approach is pretty similar to the one you chose, except that Boost.Variant also makes sure you are not forgetting to include a handling case for each of the values the variant could possibly assume (in this case, Bptr and Cptr).

Andy Prowl
  • 124,023
  • 23
  • 387
  • 451
1

Just use virtual member functions. There's no substitute for the real thing

class A {
public:
  virtual ~A(){}
  virtual void foo() = 0;
};

class B : public A {
public:
  virtual void foo() {
     std::cout << "operate on B\n";
  }
};

class C : public A {
public:
  virtual void foo() {
     std::cout << "operate on C\n";
  }
};

and pick a good C++ introductory book.

Community
  • 1
  • 1
Olaf Dietsche
  • 72,253
  • 8
  • 102
  • 198