How to reuse code between const and non-const functions that other functions call

Question

In this code example, the loop inside the two process() functions is duplicated. The only difference is that one is const and the other is not.

Is there a way to remove duplicate code so that the loop exists only once? This is just an example, but in real code, the loop is quite complex, so for maintenance reasons, I want the loop to exist once.

 #include <iostream> #include <vector> typedef unsigned int Item; typedef std::vector<Item *> Data; struct ReadOnlyAction { void action(const Item *i) { // Read item, do not modify std::cout << "Reading item " << *i << "\n"; } }; struct ModifyAction { void action(Item *i) { // Modify item std::cout << "Modifying item " << *i << "\n"; (*i)++; } }; void process(Data *d, ModifyAction *cb) { // This loop is actually really complicated, and there are nested loops // inside it three levels deep, so it should only exist once for (Data::iterator i = d->begin(); i != d->end(); i++) { Item *item = *i; cb->action(item); } } void process(const Data *d, ReadOnlyAction *cb) { // This is the same loop as above, and so the code should not be duplicated for (Data::const_iterator i = d->begin(); i != d->end(); i++) { const Item *item = *i; cb->action(item); } } void incrementData(Data *d) { // Here we modify the pointer, and need to loop through it ModifyAction incrementItem; process(d, &incrementItem); } void saveData(const Data *d) { // Here we aren't allowed to modify the pointer, but we still need // to loop through it ReadOnlyAction printItem; process(d, &printItem); } int main(void) { Data d; // Populate with dummy data for example purposes unsigned int a = 123; unsigned int b = 456; d.push_back(&a); d.push_back(&b); incrementData(&d); saveData(&d); return 0; } 

Remember that this is not a duplicate question. The following related questions and answers are different:

• 123758 - covers only simple functions that return values, while this function calls other functions, so the solutions provided there do not work for this problem.
• 23809745 - the same problem applies only to simple functions that return values, the answers to them do not work.

If I try to find the solution given in these answers, it does not work, but it looks like this:

 template <class CB> void processT(const Data *d, CB *cb) { // Single loop in only one location for (Data::const_iterator i = d->begin(); i != d->end(); i++) { const Item *item = *i; // Compilation fails on the next line, because const Item* cannot be // be converted to Item* for the call to ModifyAction::action() cb->action(item); } } void process(const Data *d, ReadOnlyAction *cb) { processT(d, cb); } void process(Data *d, ModifyAction *cb) { processT(static_cast<const Data *>(d), cb); } 

This is a simplified example, so it would be very useful if the answers could focus on the problem (how to remove a duplicated cycle from the two process() functions), rather than comments on the project - changes in the design of course, if it removes the duplicate cycle in the process.

Answer 1

I assume you care about passing const* into action.

 template<class Arg, class Data, class Action> static void process_helper(Data *d, Action *cb) { for (auto i = d->begin(); i != d->end(); i++) { Arg item = *i; cb->action(item); } } void process(Data *d, ModifyAction *cb) { process_helper<Item*>( d, cb ); } void process(const Data *d, ReadOnlyAction *cb) { process_helper<Item const*>( d, cb ); } 

Now, if you are missing C ++ 11, write a feature class:

 template<class Data>struct iterator { typedef typename Data::iterator iterator; }; template<class Data>struct iterator<const Data> { typedef typename Data::const_iterator iterator; }; template<class Arg, class Data, class Action> static void process_helper(Data *d, Action *cb) { for (typename iterator<Data>::type i = d->begin(); i != d->end(); i++) { Arg item = *i; cb->action(item); } } 

which can extend to multiple nested loops.

Answer 2

Try something like this:

 template <class IteratorType, class CB> void processT(IteratorType first, IteratorType last, CB *cb) { while (first != last) { cb->action(*first); ++first; } } void process(const Data *d, ReadOnlyAction *cb) { Data::const_iterator first = d->begin(); Data::const_iterator last = d->end(); processT(first, last, cb); } void process(Data *d, ModifyAction *cb) { Data::iterator first = d->begin(); Data::iterator last = d->end(); processT(first, last, cb); } 

Of course, in this simplified example, you can simply use std::for_each() instead:

 #include <algorithm> void process(const Data *d, ReadOnlyAction *cb) { std::for_each(d->begin(), d->end(), &cb->action); } void process(Data *d, ModifyAction *cb) { std::for_each(d->begin(), d->end(), &cb->action); } 

Answer 3

It looks like if you are doing a piece of data in a template, for example, it compiles ....

 template <class D, class CB> void processT(D d, CB *cb) { for (auto i = d->begin(); i != d->end(); i++) { auto *item = *i; // this requires c++0x eg g++ -std=c++0x cb->action(item); } } void process(const Data *d, ReadOnlyAction *cb) { processT(d, cb); } void process(Data *d, ModifyAction *cb) { processT(static_cast<const Data *>(d), cb); } 

Edit - should work without unpleasant casts, for example:

 void process(Data *d, ModifyAction *cb) { processT(d, cb); } 

Answer 4

I think the process function is like a “proxy” to invoke the appropriate actions. The processing of these parameters and, if they are const, belong to these actions. Thus, the process function can be simplified as follows (if C ++ 11 is in place):

 template<class DATA, class ACTION> void process(DATA *d, ACTION *cb){ for (auto item : *d) { cb->action(item); } }