1

I have an application which has a lot of functions which go through all the elements of a menu toolbar.

The code looks like something like this:

subMenuDefaultMenuShortcuts( ui->fileMenu );
subMenuDefaultMenuShortcuts(ui->editMenu);
subMenuDefaultMenuShortcuts(ui->windowMenu);
subMenuDefaultMenuShortcuts(ui->helpMenu);

subMenuUpdateLabels(ui->fileMenu,hierarchy);
subMenuUpdateLabels(ui->editMenu,hierarchy);
subMenuUpdateLabels(ui->windowMenu,hierarchy);
subMenuUpdateLabels(ui->helpMenu,hierarchy);

It is possible i will change this implementation, or menus could have sub menus. Thus search and replacing code, is not only ugly, but also hardly readable and error prone.

ideally i whould want something like this:

OnAllMenus(functionName,params ...)

so my code whould look like:

OnAllMenus(subMenuUpdateLabels)
OnAllMenus(subMenuUpdateLabels,hierarchy)
OnAllMenus(someFunction,hierarchy,argument1,argument2)

I wanted to use macro, but their usage is not recommended. Howerver using inline functions with function pointers seems to lead to some hardly readable code. (And i did not see any example with function pointers expecting variable number of arguments with a function).

Is there any better / cleaner way to do it without addind some overly complex unmaintanable code.

Anton
  • 1,181
  • 2
  • 17
  • 27
  • Alternatively, you can create a local array of member pointers to the menus and iterate over it. That would eliminate the repetition and thus make the code less error prone. Or even simpler write a function that takes a menu by reference an performs all of the operations on that menu, then you have the logic in a single function, and the list of menus in the caller with no need to use member pointers that are harder to read/maintain. – David Rodríguez - dribeas Jan 24 '12 at 01:30

2 Answers2

1

You can use boost::function and boost::bind.

template<typename Func>
void for_all_menus(Func f) {
  f(ui->foo);
  f(ui->bar);
  // etc
}

// use
for_all_menus(boost::bind(subMenuLabel, _1, hierarchy));

// with variadic templates
template<typename Func, typename Args...>
struct for_all_menus {
  Func f;
  void operator()(Args&&... args) {
    // umh, I always mess up the syntax
    // you might want to double check this
    f(ui->foo, std::forward<Args>(args)...);
  }
};
template<typename F>
for_all_menus<F> make_for_all_menus(F f) { return for_all_menus<F>{f}; }

// use
auto f = make_for_all_menus(subMenuLabel);
f(hierarchy);

If you need something more dynamic simply replace the function template with a function that takes a boost::function. Of course you can also use the C++11 equivalents and lambdas.

If you want to get the list of menus into one place and use that list in different places, I'd recommend Boost.Preprocessor. But you might want to think twice before resorting to it.

pmr
  • 58,701
  • 10
  • 113
  • 156
  • Note that variadic templates could be used to avoid the creation of the functor(from boost::bind). Same general idea though. –  Jan 24 '12 at 00:24
  • @EthanSteinberg Certainly. Will be added. – pmr Jan 24 '12 at 00:25
  • Of the two approaches, bind is the simplest more readable. Unless you are going to make code simpler (as in the other answer) there is no reason to use newer features just for the sake of it... This approach with variadic templates is madness – David Rodríguez - dribeas Jan 24 '12 at 01:28
1
template<typename FuncPointer, typename ... Args>
void for_all_menus(FuncPointer func, Args ... args)
{
  f(ui->foo,std::forward<Args>(args)...);
  f(ui->bar,std::forward<Args>(args)...);
  // etc
}

// use
for_all_menus(&subMenuLabel, hierarchy);

Pmr's answer, but variadic templates to stop the stupid boost::binds that will be scattered everywhere.

  • You need to use `std::forward` to make this work perfectly. But certainly cleaner than my functor. – pmr Jan 24 '12 at 00:31
  • 1
    This requires a c++11 compiler, which might or not be available. If it is available, then this is the cleanest solution, of not this solution with boost bind should not be that complicated (from @pmr answer you just need the first template (4 lines) and one call with each binded function (1 line, not *that* complex). The problem with his answer is that in providing more alternatives he has complicated the answer too much. – David Rodríguez - dribeas Jan 24 '12 at 01:25
  • works in linux , but will have to test it in windows and mac enviroments which i did not set up yet. – Anton Jan 26 '12 at 17:43