L'importanza della rimozione del codice duplicato [duplicato]

19

Ho cercato di spiegare a un collega la gravità di avere un codice duplicato in un progetto, su questo pezzo di codice:

+ (void)createIapInParse:(SKPaymentTransaction *)transaction {
    Reachability *reach = [Reachability reachabilityWithHostname:@"www.facebook.com"];
    if ([Social getFBUser]) {
        NSString *iapId = [Util getBundleNameFromIdentifier:transaction.payment.productIdentifier];
        PFObject *iap = [PFObject objectWithClassName:@"Iap"];
        iap[@"iapId"] = iapId == nil ? [NSNull null] : iapId;
        iap[@"userId"] = [Social getFBUser].objectID == nil ? [NSNull null] : [Social getFBUser].objectID;
        iap[@"email"] = [Social getFBUser][@"email"] == nil ? [NSNull null] : [Social getFBUser][@"email"];
        iap[@"country"] = [Util getDeviceCountry];
        iap[@"installationId"] = [Util getInstallationId];
        iap[@"score"] = [NSNumber numberWithLong:[GameScore getTotalScore]];
        NSTimeInterval interval = [[NSUserDefaults standardUserDefaults] doubleForKey:@"timeSpentInApp"];
        iap[@"timeSpentInApp"] = [NSNumber numberWithDouble:interval];
        iap[@"retries"] = @0;
        iap[@"transactionIdentifier"] = transaction.transactionIdentifier;
        iap[@"transactionDate"] = transaction.transactionDate;
        iap[@"transactionSource3G"] = [NSNumber numberWithBool:[reach isReachableViaWWAN]];
        [iap saveInBackgroundWithBlock:^(BOOL succeded, NSError *error) {
            NSString *query =[NSString stringWithFormat: @"...", iap.objectId, transaction.payment.productIdentifier];
            [ZeeSQLiteHelper executeQuery:query];
        }];
        NSLog(@"Save in parse: %@", iap);
    } else {
        NSString *iapId = [Util getBundleNameFromIdentifier:transaction.payment.productIdentifier];
        PFObject *iap = [PFObject objectWithClassName:@"Iap"];
        iap[@"iapId"] = iapId == nil ? [NSNull null] : iapId;
        iap[@"userId"] = @"-";
        iap[@"email"] = @"-";
        iap[@"country"] = [Util getDeviceCountry];
        iap[@"installationId"] = [Util getInstallationId];
        iap[@"score"] = [NSNumber numberWithLong:[GameScore getTotalScore]];
        NSTimeInterval interval = [[NSUserDefaults standardUserDefaults] doubleForKey:@"timeSpentInApp"];
        iap[@"timeSpentInApp"] = [NSNumber numberWithDouble:interval];
        iap[@"retries"] = @0;
        iap[@"transactionIdentifier"] = transaction.transactionIdentifier;
        iap[@"transactionDate"] = transaction.transactionDate;
        iap[@"transactionSource3G"] = [NSNumber numberWithBool:[reach isReachableViaWWAN]];
        [iap saveInBackgroundWithBlock:^(BOOL succeded, NSError *error) {
            NSString *query =[NSString stringWithFormat: @"...", iap.objectId, transaction.payment.productIdentifier];
            [ZeeSQLiteHelper executeQuery:query];
        }];
        NSLog(@"Save in parse: %@", iap);
    }
}

L'ho refactored per:

 + (void)createIapInParse:(SKPaymentTransaction *)transaction {

    Reachability *reach = [Reachability reachabilityWithHostname:@"www.facebook.com"];
    NSString *iapId = [Util getBundleNameFromIdentifier:transaction.payment.productIdentifier];
    PFObject *iap = [PFObject objectWithClassName:@"Iap"];
    NSTimeInterval interval = [[NSUserDefaults standardUserDefaults] doubleForKey:@"timeSpentInApp"];

    iap[@"iapId"] = iapId == nil ? [NSNull null] : iapId;
    iap[@"country"] = [Util getDeviceCountry];
    iap[@"installationId"] = [Util getInstallationId];
    iap[@"score"] = [NSNumber numberWithLong:[GameScore getTotalScore]];
    iap[@"timeSpentInApp"] = [NSNumber numberWithDouble:interval];
    iap[@"retries"] = @0;
    iap[@"transactionIdentifier"] = transaction.transactionIdentifier;
    iap[@"transactionDate"] = transaction.transactionDate;
    iap[@"transactionSource3G"] = [NSNumber numberWithBool:[reach isReachableViaWWAN]];

    if ([Social getFBUser]) {
        iap[@"userId"] = [Social getFBUser].objectID == nil ? [NSNull null] : [Social getFBUser].objectID;
        iap[@"email"] = [Social getFBUser][@"email"] == nil ? [NSNull null] : [Social getFBUser][@"email"];
    } else {
        iap[@"userId"] = @"-";
        iap[@"email"] = @"-";
    }

    [iap saveInBackgroundWithBlock:^(BOOL succeded, NSError *error) {
        NSString *query =[NSString stringWithFormat: @"...", iap.objectId, transaction.payment.productIdentifier];
        [ZeeSQLiteHelper executeQuery:query];
    }];

    NSLog(@"Saved in parse: %@", iap);
}

e ha continuato a discutere con me dicendo che è la stessa cosa.

Il suo argomento principale era che "è un buon programmatore e può capire e leggere anche la prima versione abbastanza velocemente", così non gli importa se è scritto così o meno.

La domanda è: ho davvero torto? Non è così importante per tutti? È una questione soggettiva o non capisce che non è la stessa cosa?

    
posta Iulian Onofrei 16.09.2014 - 11:34
fonte

6 risposte

32

Non hai torto. È psicologicamente molto difficile convincere le persone dei propri limiti.

Il motivo per cui abbiamo inventato le massime, le linee guida ecc. che limitano ciò che dovremmo fare è che, nel corso del tempo, abbiamo riscontrato che comportarsi in un modo particolare porta ad avere più successo. Soprattutto, porterà a un maggiore successo anche se non ci sembra così al momento .

Se ci pensi per un po ', ti renderai conto che è così per la maggior parte se non tutte le massime. Le cose corrette che sono ovviamente corrette sono facili da ottenere. Non ci sono massime sul non usare nomi di variabili di 500 lettere perché praticamente tutti possono vedere immediatamente che sarebbero davvero difficili da usare. Ma il codice duplicato sembra non ha alcun effetto negativo all'inizio; è facile incollare la cosa e risparmiare tempo a scomporla. Cosa non va?

Una volta che hai incontrato questo tipo di situazione, diventa abbastanza ovvio che il refactoring farà risparmiare tempo complessivo, perché sei stato su questa strada e sai che cosa succederà alla fine. Qualcuno che non ha non è così facilmente convinto. È estremamente facile ingannare te stesso nel pensare "Questo è così ovvio, non dimenticherò di occupartene, mai". (In effetti, attribuire la perfetta abilità a se stesso può essere la funzione principale della mente, secondo alcune teorie.)

Quindi l'unico modo per convincere qualcun altro a osservare le buone pratiche è guadagnare la loro fiducia o accompagnarle attraverso le stesse strade che ti hanno convinto. Il primo è una questione di abilità interpersonale, ed è sfortunatamente troppo complesso per rispondere in questo formato. Quest'ultimo implica l'attraversamento del codice dove si verifica esattamente questo problema; nella mia esperienza questo è molto più convincente quando è fatto con il codice esistente in cui la copia e incolla ha porta a errori (il più serio è il migliore) rispetto a puramente ipotetici scenari futuri.

Soprattutto, evita di attaccare direttamente il senso di autostima di qualcuno. Quasi tutti i giovani programmatori pensano di poter fare ciò che nessun altro può fare e non hanno bisogno dei dispositivi di sicurezza di cui altri hanno bisogno. Dicendo loro "si, lo fai!" funziona raramente; è marginalmente meglio mostrare loro la prova di quante persone hanno incasinato proprio per questo motivo e chiedere loro di mostrare rispetto per chiunque sia tenuto a mantenere il loro codice in futuro. (Potreste essere sorpresi dal fatto che i fattori del fattore umano abbiano un ruolo così importante nella codifica produttiva, ma sono arrivato a realizzare sempre più quanto siano decisivi, e questa è una delle ragioni per cui la costruzione di sistemi complessi è così ingannevolmente difficile).

    
risposta data 16.09.2014 - 11:48
fonte
20

His primary argument was that "he's a good programmer and he can understand and read even the first version fast enough" so he doesn't care if it's written like that or not.

Se questa è la sua argomentazione principale, potrebbe essere un segno che ha principalmente paura di non essere chiamato un buon programmatore, o anche un programmatore cattivo. IMHO il punto cruciale qui è di chiarirgli che è non un affronto personale .

Successivamente, i bravi programmatori rimangono buoni programmatori imparando nuovi trucchi. Vorrei provare a relazionarmi a questo, magari raccontando una storia dalle tue esperienze personali che lo mostrano, che i bravi programmatori possono diventare ancora migliori imparando qualcosa o ottenendo una visione specifica. Abbiamo avuto tutti questi momenti, ne sono abbastanza sicuro.

Re l'argomento "lettura": ci vuole sicuramente meno tempo per leggere solo metà del codice, vero? Quindi, se fosse solo per questo, si può facilmente aumentare l'efficienza dimezzando la quantità di codice da leggere! È bello o cosa?

Non ultimo, non si tratta solo di leggere, ma di mantenere duplicati di codice, che per definizione sono inclini all'errore. I duplicati di codice tendono a "andare alla deriva" (o avvizzire, se vuoi) via l'uno dall'altro nel tempo. Non è intenzionale, ma le persone tendono a trascurare le cose, a cambiare solo uno dei duplicati, ecc.

The question is, am I really wrong? Is this not so important for everyone? It's a subjective matter or he doesn't understand that's not the same thing?

Hai ragione. Quello che è soggettivo è lui , non tu. Il suo argomento principale si riferisce principalmente a se stesso, sei (si spera) che abbia in mente il successo del progetto.

    
risposta data 16.09.2014 - 12:10
fonte
12

I refactored it to [...] and he kept arguing with me that it's the same thing.

Bene ... per quanto riguarda il compilatore, probabilmente lo è. Per quanto riguarda lo sforzo di manutenzione, non lo è.

His primary argument was that "he's a good programmer and he can understand and read even the first version fast enough" so he doesn't care if it's written like that or not.

Questo è vero (capire che il codice non è quello difficile). Fa anche due presupposti: che il maggior sforzo richiesto in futuro su questo codice è quello di capirlo (non lo è), e che il codice scritto è corretto e non avrà bisogno di modifiche importanti (lo farà).

Il maggior sforzo in futuro, sarà probabilmente richiesto per implementare le modifiche attraverso la base di codice.

Attualmente sto lavorando a un progetto di conversione di medie dimensioni (adattare ~ 100 progetti Visual Studio a Win64). Quando trovo un errore nel codice, di solito lo trovo tra 20+ e 40+ posti (quindi correggere errori uno richiede ore di ricerca nel codice base, lettura del codice, applicazione della stessa correzione più e più volte e ricompilare). Questo rende i commit più grandi

Il codice non è solo duplicato, è proliferato (perché questa scusa una tantum di "è più semplice incollare il codice che rifattorizzare" è stata applicata con giudizio per gli ultimi 20 anni).

Questo rende molto difficile:

  • copre tutti i percorsi del codice per una correzione
  • stimare lo sforzo per correggere qualcosa
  • determina cosa testare per verificare un cambio di codice

Aumenta anche drasticamente il tempo / complessità di tutte le operazioni eseguite sul codice (a causa dell'elevato grado di duplicazione, i file sorgente sono più grandi, la compilazione richiede più tempo, la ricerca / sostituzione richiede più tempo, la lettura del codice richiede più tempo e così via ).

Un altro svantaggio è che è difficile (al punto di non essere pratico a questo punto) implementare i test unitari e documentare o formalizzare interfacce e responsabilità (perché invece di avere dipendenze tra cinque API distinte e testabili, abbiamo venticinque API che eseguono all'incirca le stesse operazioni con codice copiato da una all'altra e lievi variazioni).

Questo punto (a difficoltà di aumentare la qualità del codice ora) mantiene l'intera squadra a recuperare le scadenze, a lavorare in modo reattivo e impossibilitato a fissare obiettivi a lungo termine nel progetto.

    
risposta data 16.09.2014 - 12:25
fonte
5

Hai ragione.

"Quindi, cosa succede se qualcuno altro di te ha bisogno di aggiungere un campo a iap ? Si ricorderanno di aggiungerlo a entrambi i rami?"

Essenzialmente, questa è una domanda del fattore di bus - se c'è una sola persona che può mantenere efficacemente il codice, allora hai un problema perché non saranno necessariamente in giro per sempre, sia che siano effettivamente colpiti da un autobus, perché decidono di passare a nuovi pascoli, perché la direzione decide che un dipendente non dovrebbe più essere impiegato dalla società, o solo perché c'è un bug critico che deve essere risolto mentre il dipendente è in vacanza. Idealmente, il codice dovrebbe essere gestibile da qualsiasi sviluppatore che abbia familiarità con il codice, non solo da un esperto in particolare su quel bit di codice.

    
risposta data 16.09.2014 - 11:42
fonte
3

Un problema molto importante, ma sottile, quando si cerca di scrivere e mantenere la correttezza è riconoscere la differenza tra le cose che si incontrano, rispetto a cose che sono "le stesse". Ciò si verifica frequentemente nella programmazione orientata agli oggetti (il 99% delle domande relative alla clonazione profonda e all'uguaglianza profonda derivano dall'incapacità di distinguere quando è appropriato avere cose separate che corrispondono, anziché avere due riferimenti alla stessa cosa), ma il concetto si estende a codice sorgente.

La duplicazione del codice è buona nei casi in cui dovrebbe essere possibile cambiare un pezzo di codice senza influenzare l'altro. È brutto nei casi in cui i due pezzi di codice potrebbero cambiare, ma dovrebbe sempre fare la stessa cosa. Tieni presente che non importa se il codice in questione è lungo o corto . Sfortunatamente, ci sono molti casi in cui i costrutti linguistici non consentono di scomporre in modo efficiente frammenti di codice comuni, poiché l'unico modo per definire un metodo che abbia accesso alle variabili di un ambito è creare una chiusura; anche se nessun riferimento alla chiusura non dovrebbe mai sfuggire al contesto in cui è stato creato, molte lingue non hanno modo di specificarlo.

Quindi, è necessario invocare un altro principio che è applicabile al codice sorgente proprio come per gli oggetti run-time: la distinzione tra identità e matching è rilevante solo quando le cose sono mutabili o incapsulano lo stato mutabile. La decisione se duplicare segmenti di codice "immutabili", a differenza di quelli che potrebbero cambiare, dipende dalla lunghezza. Naturalmente, i segmenti di codice che sono lunghi e potrebbero cambiare sono i primi candidati alla fattorizzazione per due motivi.

    
risposta data 16.09.2014 - 16:02
fonte
-2
  1. Aggiungi un altro campo a PFObject .
  2. Imposta il valore su quel campo solo nella clausola quindi della versione originale.
  3. Esegui l'applicazione in modo da rendere createIapInParse inserire la clausola else .
  4. Compila un bug che segnala che il tuo nuovo campo non è stato impostato.
  5. Assegna la segnalazione di bug al tuo collega.
risposta data 16.09.2014 - 13:19
fonte

Leggi altre domande sui tag