Thinking that we have to start somewhere, I took it upon myself to refactor a data holder class

Qui è dove è andato storto - hai apportato una modifica architettonica alla base di codice in modo tale che diventasse molto diversa da ciò che ci si aspettava. Hai sorpreso i tuoi colleghi. La sorpresa va bene solo se coinvolge una festa, il principio della sorpresa minima è d'oro (anche se coinvolge le parti, allora saprei indossare mutande pulite!)

Ora, se pensavi che questo cambiamento fosse utile, sarebbe stato molto meglio delineare lo pseudocodice che mostrava il cambiamento e presentarlo ai tuoi colleghi (o almeno a chiunque avesse il ruolo più vicino all'architetto) e vedere cosa avevano pensato prima fare qualsiasi cosa Così com'è, sei diventato un ladro, pensavo che l'intero codice base fosse tuo con cui giocare come pensavi. Un giocatore della squadra, non un giocatore di squadra!

Potrei essere un po 'duro con te, ma sono stato nel posto opposto: controlla un codice e pensa "WTF è successo qui ?!", solo per trovare il nuovo ragazzo (è quasi sempre il nuovo ragazzo) ha deciso di cambiare un carico di cose in base a ciò che pensa che dovrebbe essere la "strada giusta". Viti con la cronologia SCM anche questo è un altro grosso problema.

As luck would have it, a team mate of mine was working on another module and happened to need the setters, because the values he required became available at different points in the flow.

Come descritto, non vedo cosa renda il richiesto setter al codice. Probabilmente non ne so abbastanza del caso d'uso, ma perché non aspettare che tutti i parametri siano noti prima di istanziare l'oggetto?

In alternativa, se la situazione richiede setter: offri un modo per bloccare il tuo codice a questi setter lancia un errore di asserzione (a.k.a. errore del programmatore) se provi a modificare i campi dopo che l'oggetto è pronto.

Penso che rimuovere i setter e usare final sia il modo "corretto" per farlo, oltre a rafforzare l'immutabilità, ma è davvero ciò che dovresti passare il tuo tempo con?

Suggerimenti generali

Vecchio = cattivo, nuovo = buono, a modo mio, a modo tuo ... questo tipo di discussione non è costruttivo.

Attualmente, il modo in cui viene utilizzato il tuo oggetto segue alcune regole implicite. Questo fa parte delle specifiche non scritte del tuo codice e il tuo team potrebbe pensare che non ci sia molto rischio per altre parti del codice di usarlo in modo scorretto. Quello che vuoi fare qui è rendere la regola più esplicita con un Builder e controlli in fase di compilazione.

In qualche modo il refactoring del codice è legato alla finestra spezzata teoria: ti sembra giusto correggere qualsiasi problema mentre vederlo (o prendere nota per qualche incontro successivo di "miglioramento della qualità") in modo che il codice sia sempre piacevole da utilizzare, sicuro e pulito. Tu e il tuo team non avete la stessa idea di ciò che è "abbastanza buono", però. Quello che vedi come un'opportunità per contribuire e rendere il codice migliore, in buona fede, è probabilmente visto in modo diverso dal team esistente.

A proposito, non si dovrebbe presumere che la tua squadra sia necessariamente affetta da inerzia, cattive abitudini, mancanza di educazione, ... il peggio che puoi fare è proiettare un'immagine di un conoscente-tutto chi è lì per mostrargli come codificare. Ad esempio, l'immutabilità non è qualcosa di nuovo , probabilmente i tuoi coetanei ne hanno letto ma solo perché questo argomento sta diventando popolare nei blog non è sufficiente per convincerli a passare il tempo a cambiare il loro codice.

Dopo tutto, ci sono infiniti modi per migliorare una base di codice esistente, ma costa tempo e denaro. Potrei guardare il tuo codice, chiamarlo "vecchio" e trovare cose da sfatare. Ad esempio: nessuna specifica formale, non abbastanza asserzioni, forse non abbastanza commenti o test, non abbastanza copertura del codice, algoritmo non ottimale, troppa memoria di utilizzo, non utilizzo del "migliore" possibile schema di progettazione in ogni caso, ecc. annuncio nauseam . Ma per la maggior parte dei punti che vorrei sollevare, potresti pensare: va bene, il mio codice funziona, non c'è bisogno di passare più tempo su di esso.

Forse la tua squadra pensa che ciò che suggerisci superi il punto di rendimenti decrescenti. Anche se hai trovato un'istanza effettiva di un bug a causa del tipo di esempio che mostri (con l'eccezione) probabilmente la correggeranno solo una volta e non vorranno rifattorizzare il codice.

Quindi la domanda è su come passare il tuo tempo e la tua energia, dato che sono limitati ? Ci sono continue modifiche apportate al software, ma generalmente la tua idea inizia con un punteggio negativo finché non puoi fornire buoni argomenti. Anche se hai ragione sul vantaggio, non è un argomento sufficiente da solo, perché finirà nel cestino " Bello da avere, un giorno ... ".

Quando presenti i tuoi argomenti, devi fare alcuni controlli di "sanità mentale": stai cercando di vendere l'idea o te stesso? Cosa succede se qualcun altro ha presentato questo alla squadra? Troveresti alcuni difetti nel loro ragionamento? Stai tentando di applicare alcune best practice generali o hai preso in considerazione le specifiche del tuo codice?

Quindi, sei sicuro di non apparire come-se stai cercando di correggere gli "errori" passati della tua squadra. Aiuta a dimostrare che non sei la tua idea, ma puoi prendere un feedback ragionevolmente. Più fai così più i tuoi suggerimenti avranno l'opportunità di essere seguiti.

How can I promote the use of the Builder pattern in my team?

Non lo fai.

Se il tuo costruttore ha 3 linee di parametri, è troppo grande e troppo complesso. Nessun modello di progettazione lo risolverà.

Se hai una classe i cui dati sono disponibili in momenti diversi, dovrebbero essere due oggetti diversi, o il tuo consumatore dovrebbe aggregare i componenti e quindi costruire l'oggetto. Non hanno bisogno di un costruttore per farlo.

Sembri più concentrato sull'essere giusto che lavorare bene con gli altri. Peggio ancora, ti rendi conto che quello che stai facendo è una lotta per il potere e tuttavia non riesci a pensare a come le cose possano provare le persone la cui esperienza e autorità ti stanno sfidando.

Invertire questo.

Concentrati sul lavoro con gli altri. Inquadra i tuoi pensieri come suggerimenti e idee. Invialo a parlare attraverso le LORO idee prima di fare domande per farle riflettere sui tuoi. Evita gli scontri diretti, e sii esternamente disposto ad accettare che andare avanti con le cose nel modo in cui il gruppo (per ora) è più produttivo che combattere una dura battaglia per cambiare il gruppo. (Anche se restituisci le cose.) Fai lavorare con te una gioia.

Fai questo in modo coerente e dovresti creare meno conflitti e trovare più idee che possano essere adottate da altri. Soffriamo tutti dell'illusione che la perfetta argomentazione logica possa stupire gli altri e vincere la giornata. E se non funziona in questo modo, pensiamo che dovrebbe .

Ma ciò è in diretto conflitto con la realtà. La maggior parte degli argomenti viene persa prima che venga effettuato il primo punto logico. Anche tra persone apparentemente logiche, la psicologia supera la ragione. Convincere le persone significa superare le barriere psicologiche per essere ascoltati correttamente, e non per avere una logica perfetta.

Is there another argument for using Builders instead of the "old way"?

"Quando hai un nuovo martello, tutto sembrerà un chiodo." - questo è quello che ho pensato quando ho letto la tua domanda.

Se fossi il tuo collega, ti chiederei "perché non inizializzare l'intero oggetto nel costruttore?" Questo è ciò che è la sua funzionalità, dopo tutto. Perché utilizzare il modello del costruttore (o di qualsiasi altro modello)?

Is the change I propose even really worth it?

È difficile capire da quel piccolo frammento di codice. Forse lo è - tu e i tuoi colleghi avete bisogno di valutarlo.

Do you have any tips for better presenting such arguments when advocating trying something new?

Sì, scopri di più sull'argomento prima di discuterne. Armati di buone argomentazioni e ragionamenti prima di entrare nella discussione.

Sono stato nella situazione in cui sono stato reclutato per le mie capacità. Quando ho avuto modo di vedere il codice, beh ... era più che orribile. Quando poi provi a ripulirlo, qualcuno che è stato lì più spesso sopprime i cambiamenti, perché non ne vedono i benefici. Sembra qualcosa che stai descrivendo. È finita con me uscire da quella posizione e ora posso evolvere molto meglio in un'azienda che ha una pratica migliore.

Non penso che dovresti solo avere un ruolo insieme agli altri nella squadra perché sono lì da più tempo. Se vedi i membri del tuo team che usano cattive pratiche nei progetti in cui lavori, è una responsabilità che devi mettere in evidenza, come hai fatto tu. Se no, allora non sei una risorsa molto preziosa. Se fai puntare le cose ancora e ancora, eppure nessuno ti sta ascoltando, chiedi alla tua direzione una sostituzione o un cambiamento di lavoro. È molto importante non permettere a te stesso di adattarti alle cattive pratiche.

Alla domanda attuale

Perché usare oggetti immutabili? Perché sai cosa hai a che fare.

Supponiamo di avere una connessione db passata che ti permette di cambiare l'host tramite un setter, quindi non sai quale connessione sia. Essendo immutabile, allora lo sai.

Dì comunque che hai una struttura di dati che può essere recuperata dalla persistenza e quindi ri-persistere con nuovi dati, quindi ha senso che questa struttura non sia immutabile. È la stessa entità, ma i valori possono cambiare. Utilizza invece gli oggetti valore immutabili come argomenti.

Ciò su cui si sta incentrando la domanda è comunque un modello di builder per eliminare le dipendenze dal costruttore. Dico un grosso grasso NO a questo. L'istanza è ovviamente già complessa. Non cercare di nasconderlo, risolvilo!

Tl; dr

La rimozione dei setter può essere corretta, a seconda della classe. Il modello di builder non risolve il problema, ma lo nasconde. (La sporcizia sotto il tappeto esiste ancora anche se non puoi vederla)

Mentre tutte le altre risposte sono molto utili (più utile della mia sarà), c'è una proprietà dei costruttori che non hai ancora menzionato: trasformando la creazione di un oggetto in un processo, puoi far rispettare i complessi vincoli logici.

È possibile specificare ad esempio che determinati campi sono impostati insieme o in un ordine specifico. Puoi persino restituire un'implementazione diversa dell'oggetto target a seconda di quelle decisioni.

// business objects

interface CharRange {
   public char getCharacter();
   public int getFrom();
   public int getTo();
}

class MultiCharRange implements CharRange {
   final char ch;
   final int from;
   final int to;

   public char getCharacter() { return ch; }
   public int getFrom() { return from; }
   public int getTo() { return to; }
}

class SingleCharRange implements CharRange {
   final char ch;
   final int position;

   public char getCharacter() { return ch; }
   public int getFrom() { return position; }
   public int getTo() { return position; }
}

// builders

class Builder1 {
   public Builder2 character(char ch) {
      return new Builder2(ch);
   }
}

class Builder2 {
   final char ch;
   int from;
   int to;

   public Builder2 range(int from, int to) {
      if (from > to) throw new IllegalArgumentException("from > to");
      this.from = from;
      this.to = to;
      return this;
   }

   public Builder2 zeroStartRange(int to) {
      this.from = 0;
      this.to = to;
      return this;
   }

   public CharRange build() {
      if (from == to)
         return new SingleCharRange(ch,from);
      else 
         return new MultiCharRange(ch,from,to);
   }
}

Questo è un esempio nullo che non ha molto senso ma dimostra tutte e tre le proprietà: il carattere è sempre impostato per primo, i limiti dell'intervallo sono sempre impostati e convalidati insieme e tu scegli la tua implementazione al momento della compilazione.

È facile vedere come questo può portare a un codice utente più leggibile e affidabile ma, come puoi anche vedere, c'è un lato negativo: un sacco e un sacco di codice builder (e ho persino escluso i costruttori per abbreviare i frammenti) . Quindi provi a calcolare il trade-off, ponendo domande come:

• Realizziamo l'istanza dell'oggetto da una o due posizioni? È probabile che cambi?
• Forse dovremmo rafforzare questi vincoli rifattorizzando l'oggetto originale in una composizione di oggetti più piccoli?
• Finiremmo per passare il costruttore da metodo a metodo?
• Potremmo semplicemente utilizzare un metodo di produzione statico invece?
• Questa parte di un'API esterna deve essere infallibile e trasparente come possibile?
• Approssimativamente quanti dei nostri attuali arretrati di bug potrebbero essere evitati se avessimo dei builder?
• Quanta documentazione aggiuntiva dovremmo salvare?

I tuoi colleghi hanno semplicemente deciso che il trade-off non sarebbe stato vantaggioso. Dovresti discutere le ragioni apertamente e onestamente, preferibilmente senza ricorrere a principi astratti, ma concentrandoti sulle implicazioni pratiche di questa o quella soluzione.

Sembra che questa classe sia in realtà più di una classe, messa insieme nella stessa definizione di file / classe. Se si tratta di un DTO, dovrebbe essere possibile istanziarlo in un colpo solo e renderlo immutabile. Se non si utilizzano tutti i suoi campi / proprietà in una qualsiasi transazione, allora è quasi certamente infranto il principio di Responsabilità Unica. Potrebbe essere la scomposizione in classi di DTO più piccole, più coese e immutabili. Prendi in considerazione la lettura di Pulisci codice se non l'hai già fatto, quindi stai meglio in grado di articolare le questioni e i vantaggi di apportare un cambiamento.

Non penso che tu possa progettare codice a questo livello da parte del comitato, a meno che tu non sia letteralmente un programmatore di mob (non sembra che tu sia in quel tipo di squadra), quindi penso che sia giusto fare un cambiamento basato sul tuo miglior giudizio, e poi prendilo sul mento se si scopre che hai giudicato sbagliato.

Tuttavia, finché sei in grado di articolare i potenziali problemi con il codice così com'è, puoi ancora discutere che è necessaria una soluzione , anche se la tua non è accettata. Le persone sono quindi libere di offrire alternative.

" opinioni forti, indebolite " è un buon principio - vai avanti con sicurezza basandoti su ciò che sai, ma se trovi qualche nuova informazione che neutralizza, sii modesta e abbandona e inserisci una discussione su quale dovrebbe essere la soluzione corretta. L'unica risposta sbagliata è lasciare che peggiori.