tags:

views:

158

answers:

4

I've found myself writing some repetitious code in C++. I'm using some auto-generated, such that if I want to deal with Foo, Bar, & Baz they all have fairly similar method. E.g., get_foo, get_bar, get_baz, etc.

For each "thing" I more or less have to do the same types of thing. Check if it exists, if it does, get the log, look for the most recent entry in the log, check the entry for problems, etc.

That's lead to a fair bit of repeated code, along the lines of:

if (obj->has_foo) {
  if(obj->get_foo().has_log()) {
    Log *l = obj->get_foo().get_foo_log();
    if (!l) {
      ERROR("Foo does not have a log")
    }    
    ... 30-40 more lines of stuff ...
  }
}

if (obj->has_bar) {
  if(obj->get_bar().has_log()) {
    Log *l = obj->get_bar().get_bar_log();
    if (!l) {
      ERROR("Bar does not have a log")
    }    
    ... 30-40 more lines of stuff ...
  }
}

if (obj->has_baz) {
  if(obj->get_baz().has_log()) {
    Log *l = obj->get_baz().get_baz_log();
    if (!l) {
      ERROR("Baz does not have a log")
    }    
    ... 30-40 more lines of stuff ...
  }
}

Is there a way I could build a collection, such that each item in the collection would have the unique aspects of Foo, Bar, Baz and I could use them in a single block of code.

Forgive the Perl-eese, but something like:

foreach my $thingie ("foo", "bar", "baz") {
    if (obj->has_$thingie) {
      if(obj->get_$thingie().has_log()) {
        Log *l = obj->get_$thingie().get_$thingie_log();
        if (!l) {
          ERROR(sprintf("%s does not have a log", $thingie))
        }    
        ... 30-40 more lines of stuff ...
      }
    } 
}

Or, if that's not the right direction, how do I avoid copy/pasting/tweaking that same fundamental block 3 times?

+2  A: 

I think you'll need to roll your own indirection, possibly using std::map to store keys of "foo", "bar", and "baz" with associated values being objects with "get_log" methods (assuming you don't need a get_foo_log() on an object returned by get_foo()). In your Perl-eese, it might change to be the following:

foreach my $thingie ("foo", "bar", "baz") {
    if (obj->has($thingie)) {
        if (obj->get($thingie).has_log()) {
            Log *l = obj->get($thingie).get_log();
            if (!l) {
                ERROR(...)
            }
            ... more lines ...
        }
    }
}
Anthony Cramp
+1, even though you've gone completely a different direction than I did. Which is preferable sort of depends why these has_foo, has_bar, has_baz members exist in the first place, and hence whether it's OK to factor them away.
Steve Jessop
Interesting, I like it for cases where there aren't that many methods. I'll keep it in mind for the future. In this specific case, I'm actually using a lot of varations has_foo, get_foo, set_foo_status, get_foo_status, etc.
Bill
Probably reflects my current experience level with C++. I am familiar with polymorphism through inheritance and general templatisation, but each of these requires a standard interface. Also, I've spent a bit of time with scripting languages like Python and Groovy where method calls etc. devolve to a name lookup in a dictionary.
Anthony Cramp
+11  A: 

Sure (code untested, I might have missed some problem with member pointers and type deduction, or I might just have left bugs):

template <typename T, typename M, typename F, typename G>
void doChecks(T *obj, M has_member, F get_fn, G getlog_fn) {
    if (obj->*has_member) {
        if (obj->*get_fn().has_log()) {
            Log *l = obj->*get_fn().*getlog_fn();
            if (!l) {
                ERROR("%s does not have a log", typeid(T).name());
            }
        }
     }
}

MyObj obj;
doChecks(obj, &MyObj::has_foo, &MyObj::get_foo, &Foo::get_foo_log);
doChecks(obj, &MyObj::has_bar, &MyObj::get_bar, &Bar::get_bar_log);
doChecks(obj, &MyObj::has_baz, &MyObj::get_baz, &Baz::get_baz_log);

Obviously you could use a functor-type template parameter, but this is the closest to the perl approach without actually building a dictionary into the object and rolling your own dispatch. You could also macro-ise those calls to doChecks if you wanted, and use some token-pasting to make them a bit shorter:

#define DOCHECKS(obj, class, thing) doChecks(obj, &MyObj::has_##thing, &MyObj::get_##thing, & class :: get_##thing##log) 

DOCHECKS(obj, Foo, foo);
DOCHECKS(obj, Bar, bar);
DOCHECKS(obj, Baz, baz);

With additional preprocessor wizardry you can probably make it a loop, not sure. Look at the Boost preprocessor library or the Chaos Preprocessor.

Steve Jessop
Nice answer. When i first saw the question my mind went to templates. But I am not nearly familiar enough with them to generate an answer like this. The notion of using function types as template parameters is not something I've encountered, though, now that I have, it is quite logical and could improve areas of my own code. Thanks.
Anthony Cramp
This makes my head hurt, which is usually the sign of a correct answer in C++. Seriously, thanks -- I just need to fully wrap my brain around this.
Bill
A: 

C++ does not support dynamic methods.
All the methods are strictly defined at compile time.

But the 30/40 lines of code can be done with a function call:

 if (!l)
 {
     ERROR("Bar does not have a log");
     return;
 }    
 //... 30-40 more lines of stuff ...
 LogStuff(*l);

You are obviously using C++ in a way that seems natural to you: So I have a couple of questions:

  • What language are you coming from?
  • What are you trying to do?
Martin York
I definitely am still getting my head around "thinking" C++. (Other constructs are new me -- I'd say modern, but at this point they're fairly established I think.) My background is Scheme, Lisp, Perl, with a some C and even less C++.
Bill
A: 

Hi

Could you please explain your problem a a bit more Can obj conatain All three at a time:' i.e all three condition can be true 1) if (obj->has_foo) 2) if (obj->has_bar) 3) if (obj->has_baz)

I would suggest you to write a base class having common functionality. Drive all three foo,bar adn baz from that base class, Now using base class pointer for executing your calls. S So you need not to repeat code for all three.

sat