Domanda relativa alla leggibilità del codice

5

Mi piacerebbe sapere, è considerata una pratica comune utilizzare costruzioni come |= , && , || , != del tutto nella singola riga di codice?

per es.

hasErrors |= vi2!=null && vi2.hasErrors() || vi.hasErrors();

Cosa si può fare per rendere il codice più leggibile?

Il linguaggio di programmazione è Java, se è importante.

    
posta dhblah 20.12.2012 - 08:17
fonte

5 risposte

20

Personalmente, considererei entrambi

hasErrors = hasErrors || (vi2!=null && vi2.hasErrors()) || vi.hasErrors();

o

hasErrors = ...;
hasErrors |= (vi2!=null && vi2.hasErrors());
hasErrors |= vi.hasErrors();

più leggibile, cioè, mi sembra che ci voglia un po 'meno tempo per leggere e capire il significato.

Nota, comunque, che le tre opzioni sono non equivalenti semanticamente: In Java, | = non cortocircuito. Quindi, se hasErrors() è un'operazione costosa e non hai bisogno dei suoi effetti collaterali, la prima delle mie opzioni è probabilmente la più efficiente.

    
risposta data 20.12.2012 - 08:50
fonte
3

IMO puoi introdurre più variabili,

 hasVi2Errors = vi2!=null && vi2.hasErrors(); 
 hasViErrors = vi.hasErrors(); 
 hasErrors = hasErrors || hasVi2Errors || hasViErrors; 

Il vantaggio di seguire è facile, poiché in ogni condizione sono tutte OR o tutte ANDs . Inoltre, se in futuro è necessario aggiungere altre condizioni come vi! = NULL, è possibile modificare solo una riga, mantenendo la soluzione ancora leggibile.

    
risposta data 20.12.2012 - 09:18
fonte
0

Se si tratta di espressioni corte, va bene inserire alcune affermazioni come quelle in una riga (o anche spezzare le righe successive).

Tuttavia, preferirei creare un metodo per questo scopo, ad es.

private boolean hasErrors() {
    return hasErrors || vi2!=null && vi2.hasErrors() || vi.hasErrors();
}

Quindi una chiamata sembrerebbe if(hasErrors()) {...} . È facile da leggere e facile da individuare dove modificare il codice, se necessario. Inoltre, è riutilizzabile e facilmente testabile nei test unitari. (Soprattutto quest'ultimo punto è molto affascinante per me.)

    
risposta data 20.12.2012 - 10:33
fonte
0

Prima di tutto, il bit-or-and-and-assign |= è semplicemente ciò che stai facendo con l'espressione. Di solito è un operatore di assegnazione = , e un lettore deve capire che questa linea è diversa, ma è fondamentalmente l'obiettivo di ciò che stai facendo. Quindi è perfettamente corretto mischiarlo con altri operatori logici trovati nell'espressione.

Per quanto riguarda la questione di mettere tutto su una riga, penso che sia una questione di lunghezza. Uno o due operatori su una riga funzionano se i nomi delle variabili sono brevi e non c'è nulla di complesso:

hasErrors |= vi2.hasErrors() || vi.hasErrors() || vi3.hasErrors();

Non sono un vero asino quando si tratta di 80 linee di lunghezza, ma non vuoi superare troppo oltre questo limite:

isfuzzywuzzy |= themodulewithcats.fuzzycats.hairlegth > 5 || themodulewithcats.shaggycats.hairlength > 7 || genericContainerclass.Isuckatnamingconventions.feelsbadman();

È meglio visualizzarlo suddividendolo in più righe

isfuzzywuzzy |= themodulewithcats.fuzzycats.hairlegth > 5 || 
                themodulewithcats.shaggycats.hairlength > 7 || 
                genericContainerclass.Isuckatnamingconventions.feelsbadman();

Allineamento ideale con l'inizio dell'espressione, ma l'auto-formattazione non la cattura mai, quindi è una specie di dolore.

E se hai un mix di operatori nell'espressione, o qualsiasi cosa che lo renda più complesso, è meglio dividerlo e mettere i parametri attorno a cose che devono essere esplicite. Rompere la linea sulla logica o funziona meglio a causa del suo basso ordine di operazioni.

hasCrazyErrors |= hasErrors > someObscureCode() || 
                  (vi2!=null && vi2.hasErrors()) || 
                  vi.hasErrors(); 

E anche se non ti piacciono le parentesi implicite e le linee lunghe, tutto questo è diverso se hai a che fare con un ampio set di istruzioni di questo tipo:

if(Bit1) errors |= vi2.hasErrors() || vi.hasErrors() || vi3.hasErrors(); else errors = 0;

Sembra orribile vero? Semplicemente difficile da leggere. Ma se ne metti abbastanza insieme ottieni qualcosa in cui l'occhio può facilmente diff e dire cosa diavolo sta succedendo. (Questo è in definitiva un esempio di violazione di Do not Repeat Yourself, ma si presenta molto nella programmazione embedded.)

if(Bit1) errors |= vi1.hasErrors() || vi.hasErrors() || vi3.hasErrors(); else errors = 0;
if(Bit2) errors |= vi2.hasErrors() || vi.hasErrors() || vi4.hasErrors(); else errors = 0;
if(Bit3) errors |= vi3.hasErrors() || vi.hasErrors() || vi5.hasErrors(); else errors = 0;
if(Bit4) errors |= vi4.hasErrors() || vi.hasErrors() || vi6.hasErrors(); else errors = 0;
if(Bit5) errors |= vi5.hasErrors() || vi.hasErrors() || vi7.hasErrors(); else errors = 0;

E ricorda, se stai lavorando in una squadra, la convenzione che tutti usano è migliore della tua personale convenzione. L'uniformità è più importante.

    
risposta data 20.12.2012 - 17:22
fonte
-2

Personalmente ritengo che sia corretto usare queste costruzioni su una riga, se si usano parentesi per separare le affermazioni, ad esempio:

    private boolean hasErrors()
    {
      return ( ( hasErrors |= vi2 != null ) && ( vi2.hasErrors() ) || ( vi.hasErrors() ));
    }

In questo modo puoi vedere esattamente cosa sta facendo ciascuna istruzione e come sono collegati. È molto più semplice per individuare gli errori.

    
risposta data 20.12.2012 - 13:39
fonte

Leggi altre domande sui tag