Stile migliore per le variabili membro?

1
class awesomeClass {
    std::list<A> myList;
    //...
    void fillList();
};

awesomeClass::awesomeClass() {
    fillList();
}

void awesomeClass::fillList(){
    //...
    foreach(A a, otherList)
        myList.push_back(a); //add to list
}

Rispetto a:

awesomeClass::awesomeClass() {
    myList = fillList();
}

std::list<A>& awesomeClass::fillList() {
    //...
    std::list<A> ret;
    foreach(A a, otherList)
        ret.push_back(a); //add to list
    return ret;
}

Questa è solo una classe con std::list come variabile membro e una funzione che riempie questo elenco. Il primo lo riempie all'interno della funzione e il secondo restituisce un elenco e lo assegna all'elenco dei membri.

Quale di questi è uno stile migliore? Quello che riempie la lista all'interno della funzione o quella che restituisce una lista? O c'è una soluzione ancora migliore?

    
posta Davlog 28.11.2013 - 18:23
fonte

3 risposte

3

Nella programmazione orientata agli oggetti, le classi hanno

  • Stato, che è rappresentato da variabili di istanza e
  • Metodi, che cambiano lo stato

Quindi il tuo primo approccio è corretto in cui chiami un metodo che imposta la variabile di istanza.

Devi restituire un valore solo quando non è rappresentato da alcuna variabile di istanza o è un metodo pubblico chiamato dal consumatore, che non ha accesso alla variabile di istanza.

    
risposta data 28.11.2013 - 21:33
fonte
1

Qual è lo scopo e la durata dei dati che stai inserendo in questo elenco? Ha bisogno di vivere per l'intera vita di un'istanza di awesomeClass ?

Stai cercando di evitare di rigenerare di frequente l'elenco - vale a dire che c'è un valore nella memorizzazione nella cache dei dati nelle singole istanze? O sei semplicemente pigro e vuoi dichiarare tutte le tue variabili nella definizione della classe?

Consente di rendere più tangibile il nostro membro teorico:

// 2Kb+ structure
struct Data {
    char m_vendorString[1024];
    char m_productString[1024];
    uint32_t m_vendorId;
    uint32_t m_productTypeId;
    uint32_t m_productId;
};

Consente di provare alcune varianti del modo in cui possiamo incapsulare un'interfaccia da cui recuperare un numero elevato di questo oggetto di grandi dimensioni, sotto forma di estrazione da un database:

class DatabaseA1 {
    std::vector<Data> m_productQuery;
    std::vector<Data> m_vendorQuery;
public:
    // .. various functions
    const std::vector<Data>& getVendorProducts(uint32_t vendorId);
    const std::vector<Data>& getProductVendors(uint32_t productTypeId);
};

class DatabaseA2 {
    std::vector<Data> m_results;
public:
    // .. various functions
    const std::vector<Data>& getVendorProducts(uint32_t vendorId);
    const std::vector<Data>& getProductVendors(uint32_t productTypeId);
};

class DatabaseB {
public:
    // .. various functions
    std::vector<Data> getVendorProducts(uint32_t vendorId);
    std::vector<Data> getProductVendors(uint32_t productTypeId);
};

class DatabaseC {
public:
    // .. various functions
    bool getVendorProducts(std::vector<Data>& into, uint32_t vendorId);
    bool getProductVendors(std::vector<Data>& into, uint32_t productTypeId);
};

Considera che facciamo qualcosa come il seguente pseudo-codice:

Database db(parameters);

for (possibly hours or days) {
    // get user input
    if (option == GetVendorProducts)
        getVendorProducts(id);
    else if (option == GetProductVendors)
        getProductVendors(id);
    else ...
    doOtherStuff();
}

Diciamo che i set di risultati contengono alcune migliaia di voci di dati, quindi pochi Mb di dati.

I dati continueranno a persistere fino a quando non saranno fuori portata o finché non chiami qualcosa per rilasciarlo.

Nel caso di A1 avrai questo problema per ogni tipo di query - se non fornisci un meccanismo per rilasciare i vettori, e qualcuno interroga i prodotti e quindi i venditori, avrai 2x numero di Mbs di memoria utilizzati. Nel caso di A2, ti sei obbligato a conservare informazioni su quali dati stai conservando.

Per A1 e A2 dovrai aggiungere una funzione di accesso che dirà ai membri di rilasciare la loro memoria, che è poco pratica e aumenta l'ingombro della tua API.

Nel caso di B, il compilatore può utilizzare l'ottimizzazione del valore di ritorno (RVO) per renderlo efficiente.

Nel caso di C, eviti la dipendenza da RVO e rendi molto più facile riutilizzare in modo affidabile un vettore fornito dall'utente finale.

Ci sono momenti in cui è richiesto un pattern come A1 o A2, se è probabile che tu debba ricostruire i dati un sacco di volte, e se gli oggetti stessi dipendano effettivamente dai dati (che non è la stessa cosa semplicemente avendo un sacco di funzioni di supporto che possono lavorare con i dati).

Un esempio, ovviamente, sarebbe se si tentasse specificamente di incapsulare un insieme di dati dietro un'astrazione, ad es.

class DatabaseResults {
    DatabaseQueryType m_queryType;
    std::string m_srcQuery;
    std::vector<Data> m_results;
public:
    bool populateResults(DatabaseQueryType queryType, std::string srcQuery);
    const std::vector<Data>& getResults() const { return m_results; }
};

Qual è il rapporto di set di risultati con il resto della nostra classe? Se è solo un output, probabilmente non lo vogliamo nella classe. Se restituiamo un riferimento ad esso, quali promesse facciamo ai chiamanti riguardo alla sua durata.

class Fnar {
    std::list<ImportantThings> m_list;
public:
    void makeAList();
    std::list<ImportantThings>& getThings() { return m_list; }
    void mowLawn() {
         if (m_list.empty())
             crashHorribly();
         m_list.clear();
         std::cout << "lawn mowed\n";
    }
};

Fnar fnar;
fnar.makeAList();
auto& list = fnar.getThings();

// either
list.clear(); // take that, fnar.
list.mowLawn(); // horrible crash happens here.

// or
list.mowLawn(); // horrible crash happens here.
for (auto& it: list) {
    std::cout << it << "\n";
}

Nessuno dei due casi funziona correttamente.

Se fnar non usa mai la lista stessa, allora dovremmo davvero sollevare la lista e lasciare che sia la proprietaria a chiamarla. Se c'è un altro valore per mantenere l'elenco - forse stiamo persistendo qualche concetto di proprietà su alcuni dati down-stream che abbiamo usato per costruire l'elenco - allora c'è valore nell'avere l'elenco in classe e nel fornire ai chiamanti un const -solo vista di esso:

    const std::list<ImportantThings>& getThings() const { return m_list; }

Cerca di evitare di costruire empire in classi - è facile lanciare tutto nell'istanza in modo che nel debugger puoi vedere tutte le variabili che sono state utilizzate per raggiungere lo stato corrente o devi serializzarle in un secondo momento e mantenere tutto quello stato .

Se non lo fai, allora possiedi solo ciò di cui hai assolutamente bisogno. Renderà il codice più semplice e aiuterà a localizzare le risorse che renderanno più facile comprendere il loro ruolo nel codice.

    
risposta data 29.11.2013 - 09:31
fonte
0

Se tutto quello che stai facendo è copiare un elenco, utilizzerei il costruttore dell'intervallo.

c ++ 11

AwesomeClass::AwesomeClass (const std::list &otherList) 
: myList (std::begin (otherList), std::end (otherList))
{
}


C ++

AwesomeClass::AwesomeClass (const std::list &otherList) 
: myList (otherList.begin (), otherList.end ())
{
}
    
risposta data 29.11.2013 - 22:19
fonte

Leggi altre domande sui tag