Quanto è accettabile il refactoring? [duplicare]

Non mi concentrerei sulla quantità, ma piuttosto sulla sostanza. Il numero di file interessati durante un gruppo di operazioni di refactoring è irrilevante quanto il LOC che scrivi al giorno. Un esempio estremo è la ridenominazione dei metodi per seguire una convenzione. Può influenzare migliaia di file, ma è appena più importante di un refactoring focalizzato su due file che cambiano radicalmente il loro design.

Concentrandosi su quali sono il gruppo di operazioni di refactoring che migliorano piuttosto che quanto file o LOC sono interessati, si riduce anche la complessità del problema che si sta incontrando attualmente . Durante la revisione del codice, esaminare l'impatto complessivo di tali operazioni. Migliorano la base del codice? Quanto?

Considerare le operazioni di refactoring singolarmente. Ciò consentirà al tuo team di determinare con precisione ciò che è considerato utile e cosa dovrebbe essere evitato. Ad esempio, immagina che durante una revisione del codice, noti che il tuo collega ha effettuato le seguenti operazioni di refactoring:

1. Dividi i metodi che stavano facendo troppo,

2. Modello di fabbrica astratto introdotto,

3. Sostituito alcuni if s per ereditarietà, semplificando la logica,

4. Sostituite alcune ricorrenze di ereditarietà di if s, riducendo il numero di classi e LOC e rendendo il codice più facile da leggere,

5. Riscrivi il metodo findSparse per essere più leggibile,

6. Rinominati alcuni metodi per mostrare che quelli sono effettivamente getter e setter,

7. Sostituito un singleton e introdotto Dependency Injection: il codice ora è molto più facile da testare.

Invece di dire che sei modifiche hanno interessato 28 file e dovrebbero essere ignorati, poiché sono sopra la soglia di 25 file, la tua squadra potrebbe piuttosto considerare che:

• Le operazioni di refactoring 1 e 7 ne valgono la pena.

• Le operazioni 2, 3 e 4 sono interessanti, ma costano molto tempo. Non dovrebbero essere fatti se il progetto è sotto pressione, ma dovrebbe essere altrimenti.

• L'operazione 6 è cosmetica e presenta il rischio di introdurre bug nella parte del progetto che utilizza Reflection (spiegare i rischi che superano i benefici è essenziale). Questa operazione dovrebbe essere posticipata fino a quando tutta la squadra è pronta per effettuare la modifica.

• L'operazione 5 non sta migliorando nulla: l'autore del refactoring ha appena riscritto il codice per adattarlo al suo stile. La maggior parte dei membri del team concorda sul fatto che questo non rende il codice più leggibile, e alcuni sostengono che effettivamente trovano la versione originale più facile da capire. Operazioni simili dovrebbero essere evitate in futuro.

Concentrandoti sulle singole operazioni, rendi molto facile per il tuo collaboratore capire il punto di vista della squadra, cosa è il benvenuto e cosa no. Concentrandoti sulla quantità, la fai più "Non mi piace il tuo modo di lavorare" le critiche non costruttive.

Nota che:

• Se i commit sono abbastanza granulari (e dovrebbero esserlo, per il refactoring sopra elencato, mi aspetto almeno sette commit, e l'ottavo uno che implementa la funzionalità stessa, idealmente, anche il punto 3 e il punto 4 dovrebbero essere soggetto a più commit, che porta a più di dieci commit in generale ), il rollback di un commit non dovrebbe causare troppi problemi.

• Se i commit sono sparsi e monolitici (ovvero uno o due commit per le operazioni di refactoring sopra elencate), potrebbe esserci un problema più serio nella tua squadra. Prova a spiegare loro che un commit dovrebbe corrispondere a una modifica, il più piccolo possibile, pur rimanendo indipendente da altre modifiche.

Accordati in anticipo per quanto riguarda il refactoring su ogni biglietto. Questo è fatto come una squadra. Se lo sviluppatore vuole fare cose extra, portalo su e decidi se va bene. Mantieni le cose con precisione. Non c'è niente di peggio nel cercare di spiegare a un boss perché il team ha rotto qualcosa che non era nel campo di applicazione.

Inoltre, verifica che il refactoring stia effettivamente migliorando la base del codice. Il tuo team avrebbe dovuto eseguire alcune metriche del codice sul tuo sistema identificando gli hotspot. Ad alcuni sviluppatori piace solo riscrivere tutto, assicurarsi che gli sforzi stiano effettivamente migliorando la base di codice. A volte non vale lo sforzo o il lavoro svolto non sta effettivamente migliorando nulla. Ancora una volta, la misurazione prima e dopo aiuterà.

Loda i membri della squadra che fanno il lavoro extra. È positivo che siano orgogliosi dei miglioramenti. Prova a mostrare loro che vogliamo catturare tutto il buon lavoro che stanno facendo e non creare sorprese. Potrebbero resistere alla definizione dell'ambito, ma uno sicuramente non vuole un mucchio di cannoni allentati nella base di codice perché a un certo punto potrebbe tornare a morderti.

how many files should one be touching at max?

In teoria, puoi procedere al refactoring a tempo indeterminato perché non esiste una base di codice perfetta.

Un modo per aggirare ciò che ha funzionato per me e il mio team è identificare queste aree che necessitano di pulizia, fare dei biglietti fuori da loro (chiamatele biglietti del debito tecnico o qualsiasi altra cosa lungo quelle linee) quindi durante la pianificazione dello sprint allega questi biglietti del debito tecnico al tuo biglietto principale.

In questo modo stai definendo il tuo lavoro e non eseguendo ulteriori refactoring che potrebbero causare problemi ad altri sviluppatori (dato che non stai calpestando le dita dei piedi).

Nell'approccio sopra è ovvio che non catturerai ogni singola, piccola area di codice per il refactoring come debito tecnologico, quindi per quei piccoli cambiamenti puoi includerli tutti sotto il ticket su cui stai lavorando. Dovrebbe andare bene.

Tuttavia, piccole modifiche non dovrebbero aggiungere più di 20 file rispetto a quanto ti aspettavi , e se lo fai vedere allora significa 2 cose:

1) La persona sta facendo WAY troppe cose casuali non correlate al ticket.

2) La persona sta facendo un grande refactoring che non è stato catturato in precedenza.

In entrambi i casi puoi definitivamente mettere in discussione questi due scenari.

Per il primo caso puoi dire quanto segue:

Fare un bel po 'di modifiche non correlate al ticket è più distraente che utile in termini di lasciare il tuo codice base più pulito di quanto tu non abbia trovato .

Questo mantra riguarda il codice al quale stai lavorando attualmente , quindi se lavori con la classe A ed è legato alla classe B che è un po 'caotica, allora il refactoring avrebbe senso.

Ma se stai lavorando con la classe A e stai rifattando la classe Z che è totalmente scollegata, non solo stai creando più bisogno di test, ma stai anche distraggando e rallentando il processo di sviluppo generale (rallentando i test di sviluppo, commutazione di contesto non necessaria, più risorse di test, introduzione di maggiore entropia nel flusso di lavoro).

Per il secondo caso puoi dire quanto segue:

Lavorare su una grande quantità di debito tecnologico che non è stato catturato in precedenza è un tradimento del processo e del team poiché non è stato concordato da tutti e non è stato considerato prioritario dagli analisti di business come qualcosa da spendere tempo / risorse on.

Per la prossima volta che si trova una grossa fetta di debito tecnico, si dovrebbe catturarlo e aggiungerlo all'arretrato per una successiva definizione delle priorità invece di affrontarlo senza pianificazione e direzione.

Per me, sembra che tu e il tuo compagno di squadra non siete sulla stessa pagina quando si tratta del livello di qualità richiesto per il progetto.

Recentemente ho avuto lo stesso problema con una squadra. Ciò che ha funzionato per me è stato determinare il design ideale su cui eravamo tutti d'accordo per ogni parte del progetto. Il repository userebbe CQS, i controller MVC sarebbero molto sottili e utilizzerebbero principalmente il livello di servizi ecc.

Una volta che ogni membro del team è d'accordo sulla progettazione del progetto, formalizzalo, ad esempio, con alcuni diagrammi UML. Stampali in modo da averli a disposizione nella stanza della squadra durante le revisioni del codice. Quando il design è chiaro, è più facile concordare quando il bene è abbastanza buono e non è richiesto altro refactoring.

Inoltre, raccomando di refactoring solo parti del codice relative alla funzione / storia che stai attualmente lavorando. In questo modo, il tempo di refactoring è piccolo rispetto all'intera funzionalità e puoi "vendere" il tempo di refactoring più facilmente alla gestione del progetto.