Perché foo-> bar-> foobar è considerato di cattivo gusto? E come evitare senza aggiungere codice?

Question

Il nostro professore di C++ ha affermato che l'utilizzo del risultato di operatore-> come input in un altro operatore-> è considerato di cattivo gusto.

Così, invece di scrivere:

return edge->terminal->outgoing_edges[0]; 

avrebbe preferito:

Node* terminal = edge->terminal; 
return terminal->outgoing_edges[0]; 

1. Perché questo è considerato cattivo stile?
2. Come è possibile ristrutturare il mio programma per evitare lo 'stile negativo' ma evitare anche la riga di codice aggiuntiva creata come da precedente suggerimento?

Answer 1

Dovresti chiedere al tuo professore perché lo considera un cattivo stile. Io non. Tuttavia, ritengo che la sua omissione del const nella dichiarazione di terminal sia di cattivo gusto.

Per un singolo frammento di questo tipo, probabilmente non è cattivo stile. Tuttavia considerare questo:

void somefunc(Edge *edge) 
{ 
    if (edge->terminal->outgoing_edges.size() > 5) 
    { 
     edge->terminal->outgoing_edges.rezize(10); 
     edge->terminal->counter = 5; 
    } 
    ++edge->terminal->another_value; 
} 

Questo sta cominciando a ottenere ingombrante - è difficile da leggere, è difficile da scrivere (ho fatto circa 10 errori di battitura durante la digitazione che). E richiede molta valutazione dell'operatore-> su quelle 2 classi. OK se l'operatore è banale, ma se l'operatore fa qualcosa di eccitante, finirà per fare un sacco di lavoro.

Quindi ci sono 2 possibili risposte:

1. mantenibilità
2. efficienza

e un singolo frammento del genere, non si può evitare la linea supplementare. In qualcosa di simile a quanto sopra, avrebbe comportato una minore digitazione.

Answer 2

C'è una serie di motivi.

Il Law of Demeter fornisce un motivo strutturale (si noti che il codice dei professori C++ continua a violare questo però!). Nel tuo esempio, edge deve sapere su terminal e outgoing_edges. Ciò lo rende strettamente accoppiato.

In alternativa

class Edge { 
private: 
    Node* terminal; 
public: 
    Edges* outgoing_edges() { 
     return terminal->outgoing_edges; 
    } 
} 

Ora è possibile modificare l'implementazione di outgoing_edges in un unico luogo senza cambiare ovunque. Ad essere onesti, in realtà non lo compro nel caso di una struttura dati come un grafico (è strettamente accoppiato, i bordi ei nodi non possono sfuggire l'un l'altro). Sarebbe un'eccessiva astrazione nel mio libro.

C'è anche il problema di assenza di dereferenza, nell'espressione a->b->c, cosa succede se b è nullo?

Answer 3

I due frammenti di codice che hai mostrato sono (ovviamente) semanticamente identici. Quando si tratta di questioni di stile, suggerisci di seguire semplicemente ciò che vuole il tuo professore.

Leggermente meglio del tuo secondo frammento di codice sarebbe:

Node* terminal = (edge ? edge->terminal : NULL); 
return (terminal ? terminal->outgoing_edges[0] : NULL); 

Io parto dal presupposto che outgoing_edges è un array; in caso contrario, è necessario verificare che sia NULL o vuoto.

Answer 4

Beh, non posso essere sicuro di ciò che il tuo professore ha voluto dire, ma ho qualche idea.

Prima di tutto, il codice come questo può essere "non così ovvio":

someObjectPointer->foo()->bar()->foobar() 

Non si può davvero dire che cosa è il risultato di foo(), e quindi non si può davvero dire in cosa viene chiamato bar(). È lo stesso oggetto? O forse è un oggetto temporaneo che viene creato? O forse è qualcos'altro?

Un'altra cosa è se devi ripetere il codice. Si consideri un esempio come questo:

complexObject.somePart.someSubObject.sections[i].doSomething(); 
complexObject.somePart.someSubObject.sections[i].doSomethingElse(); 
complexObject.somePart.someSubObject.sections[i].doSomethingAgain(); 
complexObject.somePart.someSubObject.sections[i].andAgain(); 

confrontarlo con tale:

section * sectionPtr = complexObject.somePart.someSubObject.sections[i]; 
sectionPtr->doSomething(); 
sectionPtr->doSomethingElse(); 
sectionPtr->doSomethingAgain(); 
sectionPtr->andAgain(); 

si può vedere come il secondo esempio non è solo più breve, ma più facile da leggere.

A volte la catena di funzioni è più facile, perché non c'è bisogno di preoccuparsi di ciò che essi ritornano:

resultClass res = foo()->bar()->foobar() 

vs

classA * a = foo(); 
classB * b = a->bar(); 
classC * c = b->foobar(); 

Un'altra cosa è il debug. È molto difficile eseguire il debug di lunghe catene di chiamate di funzione, perché semplicemente non si riesce a capire quale di esse causa l'errore. In questo caso, rompendo la catena aiuta molto, per non parlare che si può fare alcuni controlli aggiuntivi anche, come

classA * a = foo(); 
classB * b; 
classC * c; 
if(a) 
    b = a->bar(); 
if(b) 
    c = b->foobar(); 

Quindi, per riassumere, non si può dire che si tratta di un "cattivo" o "buona "stile in generale. Devi prima considerare le tue circostanze.

Answer 5

Né sono necessariamente migliori nella loro forma attuale. Tuttavia, se si dovesse aggiungere un controllo per null nel secondo esempio, ciò avrebbe senso.

if (edge != NULL) 
{ 
    Node* terminal = edge->terminal; 
    if (terminal != NULL) return terminal->outgoing_edges[0]; 
} 

Sebbene anche questo è ancora un male perché chi può dire che outgoing_edges sia correttamente popolate o assegnati. Una scommessa migliore sarebbe quella di chiamare una funzione chiamata outgoingEdges() che fa tutto quel bel controllo degli errori per te e non causa mai un comportamento non definito .