Programma "Refactor This!" [Da leggere]
La code base sta pian piano migliorando, soprattutto Droppable, a seguito di un po' di colpi di refactoring. Ma durante il refactoring giornaliero ho avuto molti problemi sia con BigGem sia con vari test.
Visto che affidarsi al vostro bon cuore non sta sortendo effetti troppo visibile, la soluzione e' il programma "Refactor This!" Ho seminato nel codice questo commento: // TODO: REFACTOR THIS Il commento indica un metodo che va rifattorizzato semplicemente perche' non si capisce che cosa voglia fare, e' oscuro, o troppo lungo, o non utile. Insomma, va rifattorizzato e ripulito in un metodo piu' chiaro. Le regole di commit cambiano dunque cosi': - ad ogni commit dev'essere associata la rimozione di un TODO "Refactor This" - nella descrizione del commit indicare il file e la riga del TODO eliminato - scrivere in questo topic il codice prima e dopo il refactoring Il non seguire questa regola portera' al revert del commit. E' una soluzione temporanea un po' draconiana, ma e' chiaro che serve solo per riportare velocemente la code base ad uno stato piu' usabile, di modo da facilitare i successivi task e renderli piu' divertenti per tutti. Al momento lavorare sulla code base e' un lavoraccio ed e' molto complesso. Se sono necessarie eventuali deroghe alla regola, magari per un commit veloce, o una serie di commit (per i quali basta un solo TODO eliminato), contattatemi pure in MSN. Senza deroga vale la regola: "Un commit, un TODO eliminato". Periodicamente faro' un giro della code base per indicare i metodi da rifattorizzare. Sentitevi liberi di aggiungere il TODO ad un metodo che reputate troppo complesso o non chiaro. |
Regole generali per il refactoring:
- Quando un metodo da privato diventa pubblico bisogna aggiunger il test!! P.S.: se volete che aggiunga qualcos'altro, ditemelo ;) |
bella come idea :D
mi metto a caccia. |
oh porca :mbe:
meno male che per ora non ho tempo di committare (dopo aver finito di lavorare alle 20 e 30 finalmente mi sto prendendo un pò di riposo :Prrr: ) P.S: il mio amico giustamente aggiunge: "non direi proprio"... dato che fino ad ora ho bestemmiato per settargli l'ADSL cercando di recuperare la password del router che aveva perso :asd: |
it.diamonds.grid.action.CrushByFlashAction.java:53
codice attuale: Codice:
private Droppable getGemToDelete(Droppable falshingGem) Codice:
private Droppable getGemToDelete(Droppable falshingGem) |
mi ci sono scervellato, meglio di così non riesco a fare: non riesco ad eliminare quegli if (PER ORA). committo così com'è.
|
minchia, guardate che roba :asd:
it.diamonds.tests.droppable.gems.TestBigGemInGrid:457 Codice:
public void testBigGemMoveDownABit() Codice:
public void testBigGemMoveDownABit() |
71104, sei stato tu a mettere static le rows e le columns in Grid??
A me va benissimo, ma ciò vuol dire che tutte le grid hanno lo stesso numero di colonne e righe, e anche questo mi va benissimo, ma anche che sono inizializzate a 0 se non specificato diversamente: Codice:
Se non viene mai costruita una grid, rows e columns sono entrambi a 0. :muro: Ma poi mi speighi il vantaggio di averli statici?!? Io no sono lo spezzatore di dita ufficiale, ma poi finisce che chiedo una delega . :Perfido: :Perfido: |
Mi è venuta in mente una cosa carina...
Vediamo quanti test scoppiano se nel config cambio il valore di columns o di rows ?!? No provo perchè potrei svenire. :lamer: |
Bisogna prendere una decisione per Grid.
Tutti i metodi, secondo me, devono essere omogenei; di conseguenza: - tutti ricevono come parametro Cell; - tutti ricevono come parametro row e column; Piccola grande differenza: Cell lancia una eccezione se viene creata con parametri negativi. Per isValidCell vuol dire che non c'è bisogno di controllare i valori negativi, ma che può lanciare eccezzionida un momento all'altro. Io voto per la coppia row e column. ( tanto i metodi isValidColumn e isValidCell ci sono già). Aspetto decisioni. ;) EDIT2: Ehm... già che c'ero le ho cambiate tutte, i posto in cui si usavano vermanete le celle erano 2... in tutto il resto della code base si faceva sempre new. Secondo me ora le cose sono più pulite ;) EDTI: c'è una sovrapposizione tra il metodo inserDroppable di Grid e la classe InsertDroppableAction. Che si fa? Io ucciderei la action. ;) |
Minchia Bonfo avevamo appena iniziato a cambiare tutto in Cell e hai ri-revertato i cambiamenti fatti :D
L'idea e` usare Cell ovunque. |
Quote:
Quote:
Quale TODO hai rifattorizzato per questo commit? La regola vale per tutti. |
Voglio aggiungere un'altra cosa: non si entra in produzione fino a che non spariscono i Refactor This dalla code base. Questo ci da' anche una buona metrica per capire a che punto stiamo.
Abbiamo 16 Refactor This al momento. |
Per rilassare un attimo la regola, che effettivamente e' molto draconiana, non serve che risolviate del tutto un TODO per fare il commit, basta che ci abbiate lavorato in maniera consistente e abbiate migliorato quel pezzo di codice. Poi sta a voi decidere se il codice e' in buono stato e il TODO si puo' eliminare.
Ricordate di indicare nel commit la riga di codice dove era presente il TODO. |
Quindi se uno deve ancora rifattorizzare un pezzo di codice (sto ancora lavorando ad animation) prima fa commit dei todo da fare e poi comincia ad eliminarli? Nel mio caso bisogna ancora spostare un po' di test da TestGemAnimation a TestAnimatedSprite piu` semplificarli piu` testare roba non testata...
|
Si', e' chiaro che se stai facendo una serie di commit come me in questo momento, basta un solo TODO.
|
Quote:
|
Prima:
Codice:
protected void applyOn(Droppable gem) Ora: Codice:
protected void applyOn(Droppable gem) (Non sono molto esperto di refactoring :stordita: ) |
Quote:
|
Quote:
|
Tutti gli orari sono GMT +1. Ora sono le: 10:37. |
Powered by vBulletin® Version 3.6.4
Copyright ©2000 - 2024, Jelsoft Enterprises Ltd.
Hardware Upgrade S.r.l.