2014-09-25 10 views
8

In questo codice di esempio, il ciclo all'interno delle due funzioni process() è duplicato. L'unica differenza è che uno è const e l'altro no.Come riutilizzare il codice tra le funzioni const e non-const che chiamano altre funzioni

C'è un modo per rimuovere la duplicazione del codice, in modo che il ciclo esista solo una volta? Questo è solo un esempio, ma nel codice reale il ciclo è piuttosto complesso, quindi per motivi di manutenzione voglio solo che il ciclo esista una sola volta.

#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; 
} 

Si prega di essere consapevoli del fatto che questa non è una domanda duplicata. Le seguenti domande e risposte simili sono diverse:

  • 123758 - copre solo semplici funzioni che restituiscono valori, che tale funzione chiama altre funzioni in modo che le soluzioni indicate non non funzionano per questo problema
  • 23809745 - stesso problema, copre solo semplici funzioni che restituiscono valori, le risposte non funzionano per questo problema

Se tento la soluzione data a queste risposte, non funziona, ma si presenta così:

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); 
} 

Questo è un esempio semplificato, in modo che sarebbe molto apprezzato se risposte potrebbero concentrarsi sul problema (come rimuovere il ciclo duplicato dall'interno delle due process() funzioni) piuttosto che i commenti circa la progettazione - modifiche al design vanno bene ovviamente se rimuove il ciclo duplicato nel processo.

+0

si può non solo fare la parte data' 'della firma nel modello, in modo che la proprietà const si propaga da la funzione? – Soren

+0

Perché non specificare il tipo iteratore come uno dei parametri del modello o passare gli iteratori come parametri alla funzione? –

+1

perché una versione che utilizza const e una no? Quale esigenza di progettazione sta guidando la necessità di due versioni dello stesso ciclo? –

risposta

1

Immagino che la cosa che ti interessa sia passare un const* all'azione.

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); 
} 

Ora, se ti manca C++ 11, scrivere una classe tratti:

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); 
    } 
} 

che può estendersi a più cicli annidati.

4

provare qualcosa di simile:

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); 
} 

Naturalmente, in questo esempio semplificato, si potrebbe utilizzare std::for_each() invece:

#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); 
} 
+0

Questo potrebbe funzionare, e in parte è colpa mia per aver dovuto semplificare eccessivamente per produrre un esempio leggibile, ma il ciclo in questo esempio è in realtà di tre livelli annidati. Quindi sarebbe possibile superare gli iteratori più esterni, ma non sarebbe possibile passare quelli interiori. – Malvineous

1

Sembra che se fate parte dei dati del modello, come questo, compila ....

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 e.g. 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); 
} 

Modifica - dovrebbe funzionare anche senza il cast sgradevole, come;

+0

Effettivamente. Funziona in C++ 03 facendo 'cb-> action (* i)' invece, tuttavia è valido? Significa che stai convertendo un 'const_iterator' in una variabile non-'const' e quindi modificandola. – Malvineous

+0

Dovrebbe funzionare anche senza il cast - ho appena dimenticato di toglierlo quando ho fatto il cut-n-paste – Soren

+0

E perché la versione non '' concessa' di 'perform()' esegue il casting in 'const'? Penserei che impedisce a 'process()' di chiamare 'ModifyAction :: action()' dal momento che si aspetta un input non '' '' '' '' '' ''. –

0

Penso che la funzione process sia come un "proxy" per chiamare le azioni corrispondenti. La gestione dei parametri e se sono const è di proprietà di tali azioni. Così la funzione di processo può essere semplificato in questo modo (se C++ 11 è a posto):

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