Devo seguire uno stile di codifica errato solo per seguire le convenzioni stabilite sul mio posto di lavoro?

102

Ho lavorato al mio lavoro per circa un anno. Principalmente lavoro nella nostra interfaccia GUI che utilizza metodi da un backend C, ma generalmente non devo gestirli tranne i valori di ritorno. La nostra GUI è strutturata in modo abbastanza ragionevole, date le nostre limitazioni.

Sono stato incaricato di aggiungere una funzione alla parte della riga di comando del programma. Molte di queste funzioni sono lunghe 300 righe e sono difficili da usare. Sto cercando di raccoglierne alcuni per ottenere informazioni specifiche sugli allarmi e ho difficoltà a organizzarmi. So che sto rendendo i miei test più complicati eseguendo una singola funzione lunga.

Dovrei semplicemente tenere tutto in una funzione enorme secondo lo stile delle funzioni esistenti, o dovrei incapsulare gli allarmi nelle loro funzioni?

Non sono sicuro che sia appropriato andare contro le attuali convenzioni di codifica o se dovrei solo mordere il proiettile e rendere il codice un po 'più confuso per me stesso da scrivere.

In sintesi, sto confrontando

showAlarms(){
    // tons of code
}

contro

showAlarms(){
   alarm1();
   alarm2();
   return;
}

alarm1(){
    ...
    printf(...);
    return;
}

EDIT: Grazie per il consiglio a tutti, ho deciso che ho intenzione di progettare il mio codice fattorizzato, e poi chiedere cosa vogliono, e se lo vogliono tutto in uno posso semplicemente tagliare il mio codice fattorizzato e girarlo torna in 1 grande funzione. Questo dovrebbe permettermi di scriverlo e testarlo più facilmente anche se lo vogliono tutto il codice in una singola definizione.

AGGIORNAMENTO: sono finiti soddisfatti del codice fattorizzato e più di una persona mi ha ringraziato per aver impostato questo precedente.

    
posta Justin 12.12.2016 - 22:38
fonte

10 risposte

101

Questo è veramente tra te e i tuoi compagni di squadra. Nessun altro può dirti la risposta giusta. Tuttavia, se posso osare leggere tra le righe, il fatto che tu chiami questo stile "cattivo" fornisce alcune informazioni che suggeriscono che è meglio prenderlo lentamente. Pochissimi stili di codifica sono in realtà "cattivi". Ce ne sono alcuni che non userò, me stesso, ma hanno sempre una rima o una ragione per loro. Questo suggerisce, per me, che c'è di più nella storia di quello che hai visto finora. Chiedere in giro sarebbe una chiamata molto saggia. Qualcuno potrebbe sapere qualcosa che tu non.

Mi sono imbattuto in questo, personalmente, nella mia prima incursione nella codifica mission-critical in tempo reale. Ho visto un codice come questo:

lockMutex(&mutex);
int rval;
if (...)
{
    ...
    rval = foo();
}
else
{
    ...
    rval = bar();
}
unlockMutex(&mutex);
return rval;

Essendo lo sviluppatore OO C ++ brillante e brillante che ero, li ho immediatamente chiamati sui rischi di bug che avevano bloccando e sbloccando manualmente i mutex, invece di usare Raii . Ho insistito che era meglio:

MutexLocker lock(mutex);
if (...)
{
    ...
    return foo();
}
else
{
    ...
    return bar();
}

Molto più semplice ed è più sicuro, giusto ?! Perché richiedere agli sviluppatori di ricordare di sbloccare i loro mutex su tutto il percorso del flusso di controllo quando il compilatore può farlo per te!

Bene, quello che ho scoperto dopo è stato che c'era una ragione procedurale per questo. Dovevamo confermare che, sì, il software funzionava correttamente e che esisteva un elenco limitato di strumenti che ci era stato concesso di utilizzare. Il mio approccio potrebbe essere stato migliore in un ambiente diverso, ma nell'ambiente in cui stavo lavorando, il mio approccio avrebbe facilmente moltiplicato la quantità di lavoro necessario per verificare l'algoritmo dieci volte perché ho appena portato un concetto C ++ di RAII in una sezione di codice quello era considerato come standard che erano davvero più sensibili al modo di pensare in stile C.

Quindi quello che mi sembrava uno stile di codice cattivo, decisamente pericoloso, in realtà era ben pensato e la mia "buona" soluzione era in realtà quella pericolosa che avrebbe causato problemi lungo la strada.

Quindi chiedi in giro. C'è sicuramente uno sviluppatore anziano che può lavorare con te per capire perché lo fanno in questo modo. Oppure, c'è uno sviluppatore senior che può aiutarti a capire i costi e i benefici di un refactoring in questa parte del codice. Ad ogni modo, chiedi in giro!

    
risposta data 13.12.2016 - 18:27
fonte
84

Onestamente, credi che avere funzioni che sono difficili da usare, con 300 righe, è solo una questione di stile? Quindi seguire il cattivo esempio potrebbe mantenere il programma generale in uno stato migliore a causa della coerenza? Immagino che tu non ci creda, altrimenti non avresti fatto questa domanda qui.

La mia ipotesi è che queste funzioni siano così lunghe perché nella nostra azienda ci sono molti programmatori mediocri che si limitano a impilare le funzionalità sulle funzionalità, senza preoccuparsi della leggibilità, della qualità del codice, della denominazione appropriata, dei test delle unità, del refactoring e non hanno mai imparato a creare astrazioni appropriate. Devi decidere da solo se vuoi seguire quella mandria, o se vuoi essere un programmatore migliore.

Addendum, a causa dei commenti: "300 linee" e "difficile da usare" sono le forti indicazioni di IMHO per il codice errato. Per la mia esperienza è molto improbabile che ci sia un "motivo tecnico nascosto" per cui tale codice non può essere implementato in un modo più leggibile, paragonabile all'esempio della risposta di Cort Ammon. Tuttavia, sono d'accordo con Cort che dovresti discuterne con i responsabili del team - non per trovare motivi oscuri per cui il codice o questo "tipo di stile" non possono essere modificati, ma per scoprire come il codice può essere rifatto in modo sicuro senza rompere le cose .

    
risposta data 12.12.2016 - 23:00
fonte
42

No.

Nel libro Programmer pragmatico l'autore parla del Teoria delle finestre di blocco .

Questo stato di teoria:

Consider a building with a few broken windows. If the windows are not repaired, the tendency is for vandals to break a few more windows. Eventually, they may even break into the building, and if it's unoccupied, perhaps become squatters or light fires inside.

Or consider a pavement. Some litter accumulates. Soon, more litter accumulates. Eventually, people even start leaving bags of refuse from take-out restaurants there or even break into cars.

Nel libro l'autore scrive un parallelo tra questa teoria e il codice. Una volta trovato un codice come questo, ti fermi e ti fai una domanda simile.

E la risposta è: No.

Una volta che lasci questa finestra rotta - nel nostro caso, codice errato - questo creerà l'effetto che tutti lasceranno dietro le finestre rotte.

E un giorno il codice si ridurrà.

Fornisci una copia di Pulisci codice e Clean Coder per tutti.

Mentre sei nell'argomento, anche una copia di TDD è buona.

    
risposta data 12.12.2016 - 23:14
fonte
25

Sì, provaci. Struttura! = Stile

Stai parlando di struttura, non di stile. Le linee guida sullo stile non prescrivono (di solito) una struttura, poiché la struttura viene generalmente scelta per la sua adeguatezza per un problema specifico, non per l'appropriatezza di un'organizzazione.

Fai attenzione

Devi solo essere sicuro che non stai causando conseguenze negative in altre aree che potrebbero non esserti verificate. Ad esempio,

  • Assicurati di non complicare le diff o le unioni di codice perché hai cancellato qualsiasi somiglianza con il codice esistente.
  • Verifica che i flussi delle eccezioni risalti correttamente e che le tracce dello stack non siano inquinate da un enorme mucchio di sciocchezze illeggibili.
  • Assicurati di non esporre casualmente i punti di accesso pubblici che potrebbero causare problemi se chiamati direttamente (non metterli nella mappa e / o non esportarli).
  • Non ingombrare lo spazio dei nomi globale, ad es. se le funzioni più piccole richiedono tutte una sorta di contesto globale precedentemente dichiarato in ambito locale della funzione.
  • Assicurati di controllare i registri e chiediti se eri nel team di supporto se i registri generati dal tuo codice risultassero confusi se visti intrecciati con i registri da qualsiasi codice che non tocchi.
  • Assicurati che fai aderisca allo stile esistente, ad es. anche se utilizzano la notazione ungherese della vecchia scuola e ti fa sanguinare gli occhi, sii coerente con la base di codice generale. L'unica cosa più dolorosa della lettura della notazione ungherese è la lettura del codice che utilizza una dozzina di diversi tipi di notazione a seconda di chi ha scritto cosa.

Sii gentile

Ricorda che fai parte di una squadra, e mentre un buon codice è importante, è anche molto importante che i membri del tuo team siano in grado di mantenere le cose che hai scritto quando vai in vacanza. Cerca di attenersi a un flusso che abbia senso per le persone che sono abituate al vecchio stile. Solo perché sei il ragazzo più intelligente nella stanza non significa che dovresti parlare sopra le teste di tutti!

    
risposta data 13.12.2016 - 04:03
fonte
6

Non lo vedo come una convenzione di codice. Lo vedo come qualcuno che non capisce come scrivere codice gestibile che sia facile da capire. Vorrei suddividere il tuo codice in diverse funzioni mentre suggerisci e istruisci il tuo team sui vantaggi di farlo mentre cerchi di capire perché ritengono che le funzioni a 300 linee siano accettabili. Sarei molto interessato a sentire i loro ragionamenti.

    
risposta data 12.12.2016 - 22:59
fonte
5

La risposta è nella revisione del codice.

La vera domanda è se si trasformi in codice ben fattorizzato, sarai l'unico disposto a lavorarci?

Io, come la maggior parte delle persone qui, credo che le 300 funzioni di linea siano un abominio. Tuttavia, portare Windex in una discarica non farà molto bene. Il problema non è il codice, sono i programmatori.

Il solo fatto di avere ragione non è abbastanza Hai bisogno di vendere persone con questo stile. Almeno leggendolo. Se non finisci come questo povero ragazzo: licenziato perché le tue abilità sono troppo al di sopra dei tuoi colleghi

Inizia in piccolo. Chiedere in giro e scoprire se qualcun altro favorisce funzioni più piccole. Meglio ancora curiosare attorno al codice base e vedere se riesci a capire chi scrive i più piccoli (potresti scoprire che il codice più brutto è il codice più vecchio e gli attuali programmatori stanno scrivendo un codice migliore). Parla con loro di questo e vedi se hai un alleato. Chiedi se conoscono qualcun altro che si sente allo stesso modo. Chiedi se conoscono qualcuno che odia le piccole funzioni.

Raccogli questo piccolo gruppo e parla di apportare modifiche. Mostra loro il tipo di codice che vorresti scrivere. Assicurati che possano leggerlo. Prendi sul serio qualsiasi obiezione. Apporta le modifiche. Ottieni il tuo codice approvato. Ora hai il potere di un incontro dietro di te. Alcuni di questi e puoi produrre un documento di una pagina che accetta esplicitamente il nuovo stile che hai introdotto.

Gli incontri sono cose sorprendentemente potenti. Possono produrre influenza. Il potere è quello che userai per combattere coloro che si oppongono a questo movimento. E questo è ciò che è a questo punto. Un movimento Stai lottando per il diritto di migliorare lo status quo. Le persone temono il cambiamento. Dovrai parlare dolcemente, blandire e pungere. Ma con un po 'di fortuna ti trasformi nel fatto di essere accettato.

    
risposta data 13.12.2016 - 02:51
fonte
3

A volte devi andare con il flusso.

Come hai detto tu hai avuto il compito di implementare una funzione in una base di codice operativa esistente.

Indipendentemente dal caos, e capisco che è allettante ripulirlo. ma nel mondo degli affari non hai sempre la possibilità di refactoring code.

Direi solo di fare ciò che ti è stato chiesto di fare e andare avanti.

Se ritieni che valga la pena di refactoring o di riscrittura rispetto a quello che devi portare in discussione con il team.

    
risposta data 13.12.2016 - 02:11
fonte
3

Prima di rendere l'affermazione che le pratiche sono cattive, vorrei prima fare un passo avanti verso la comprensione del motivo per i grandi metodi e altre pratiche che potrebbero apparire altrettanto brutte.

Quindi, dovresti assicurarti che la copertura del test sia piuttosto alta o molto alta (per quanto puoi fare senza un refactoring importante). Se non lo è, lavorerei per rendere la copertura davvero alta anche se non hai scritto il codice. Questo aiuta a costruire un rapporto e il tuo nuovo team ti prenderà più sul serio.

Una volta che hai coperto queste basi, nessuno sano di mente ti sfiderà. Come elemento bonus, fai qualche micro-benchmarking e questo aggiungerà davvero al tuo caso.

Spesso, il tuo modo di esprimersi fa molto per cambiare la cultura del codice. Dare l'esempio. O ancora meglio, aspetta un pezzo di codice che puoi scrivere - refactoring e unit test da fare e viola - sarai ascoltato.

    
risposta data 13.12.2016 - 03:44
fonte
3

Questo tipo di domanda è fondamentalmente " per favore, leggi i miei compagni di squadra ". Le persone casuali su Internet non possono farlo, quindi tutte le risposte che otterrete sono opinioni personali della gente sullo stile di codifica. Il consenso è che i metodi più brevi sono preferibili, ma sembra che tu la pensi già da solo, quindi non c'è bisogno di ribadirlo.

Quindi, il codice attuale sembra utilizzare uno stile di codifica diverso da quello che preferisci. Cosa dovresti fare? Per prima cosa devi capire:

  1. Si tratta di una deliberata decisione progettuale supportata dal tuo attuale team?
  2. Vogliono che tu segua questo stile?

C'è solo un modo per scoprirlo. Chiedimi .

Potrebbero esserci più motivi per avere funzioni lunghe. Potrebbe essere perché la squadra crede che le funzioni lunghe siano migliori delle molteplici funzioni brevi. Potrebbe anche essere perché è un codice legacy e il team preferisce che il nuovo codice segua un design più pulito. Quindi la scelta o potrebbe metterti nei guai come sviluppatore, se non parli con il team e capisci il loro ragionamento.

    
risposta data 13.12.2016 - 11:12
fonte
1

Ci sono diversi livelli di "stile".

A un livello, di solito parte delle convenzioni di codifica, questo significa dove inserire spazi bianchi, righe vuote, parentesi e come denominare le cose (caso misto, underscore, ecc.).

Su un altro livello c'è il modello di programmazione, a volte cose come evitare la statica, o usare le interfacce su classi astratte, come esporre le dipendenze, magari anche evitare alcune caratteristiche del linguaggio esoterico.

Poi ci sono le librerie e le strutture in uso nel progetto.

A volte ci sarà un limite massimo per le dimensioni della funzione. Ma raramente c'è un limite minimo! Quindi, dovresti scoprire quali di queste cose considerano importanti i poteri e cercare di rispettarlo.

Devi scoprire se il team è contrario al refactoring del codice esistente, che dovrebbe essere fatto indipendentemente dall'aggiunta di nuovo codice quando possibile.

Tuttavia, anche se non vogliono rifattorizzare il codice esistente, potresti essere in grado di introdurre alcuni livelli nelle astrazioni per il nuovo codice che aggiungi, il che significa che nel tempo puoi sviluppare un codice migliore e magari migrare il codice vecchio codice come richiede manutenzione.

    
risposta data 12.12.2016 - 23:05
fonte

Leggi altre domande sui tag