Codice legacy, usciamo dal pantano!

Codice Legacy,usciamo dal pantano!

Antonio Carpentieri (@acarpe)Stefano Leli (@sleli)

giovedì 30 giugno 2011

Regole

•Tempo a disposizione: 50 minuti

• Il codice va rifattorizzato per essere conforme ai principi S.O.L.I.D.

• I test possono essere modificati


S.O.L.I.D. Principles


Single Responsibility Principle

“There should never be more than one reason for a class to change”


Interface Segregation Principle

public interface MobileDevice{ void Call(string phoneNumber); void PlayAudio(Uri uri); void PlayVideo(Uri uri); void TakePicture(); void StartRecordingAudio(); void StopRecordingAudio(); void StartRecordingVideo(); void StopRecordingVideo(); }

“Clients should not be forced to depend upon interfaces that they do not use”


Liskov Substitution Principle

“Functions that use pointers or references to base classes must be able to use objects of derived

classes without knowing it.”


Bird Watching Gamehttp://github.com/sleli/BirdWatching


http://github.com/sleli/BirdWatching


Il Gioco

•Simulatore di Bird Watching

•Una sorta di “battaglia navale”... ma qui gli assi sono 3 (gli uccelli volano)


Le Classi


Analisi del problema


Classe GameFieldResponsabilità Collaborazioni

Gestisce la collezione di BirdsDispone il campo da giocoValida il campo da giocoInizializza il campo (start)Gestisce le logiche degli shot

Classe Bird

Il refactoring verterà sulla suddivizione di responsabilità (aumentare la coesione, diminuire l’accoppiamento)

•Maggiore riuso del codice•Maggiore robusteza del programma•Minor costo del cambiamento


Passi di refactoring


Duplicazione


Duplicazione (semplice)

@Before public void Setup() { field = new GameField(10,5,3, new FieldSize(10,5,3)); }



public GameField(int width, int height, int depth, FieldSize fieldSize) { this.width = width; this.height = height; this.depth = depth; this.fieldSize = fieldSize; birds = new ArrayList<Bird>(); }



//Place the birds on the fields private void placeBirds(PlacingMode type) throws Exception { ... Location location = new Location(new Random().nextInt(this.width), new Random().nextInt(this.height)); bird.setLocation(location); if (!(bird instanceof Chicken)) bird.setHeight(new Random().nextInt(this.depth)); ... }


Eliminiamola!


Eliminiamola!@Before public void Setup() { field = new GameField(new FieldSize(10,5,3)); }



public GameField(FieldSize fieldSize) { this.fieldSize = fieldSize; birds = new ArrayList<Bird>(); }



public GameField(FieldSize fieldSize) { this.fieldSize = fieldSize; birds = new ArrayList<Bird>(); }

//Place the birds on the fields private void placeBirds(PlacingMode type) throws Exception {... Location location = new Location(new Random().nextInt(fieldSize.width()), new Random().nextInt(fieldSize.height())); bird.setLocation(location); if (!(bird instanceof Chicken)) bird.setHeight(new Random().nextInt(fieldSize.depth())); ... }


Diamo a Cesareciò che è di Cesare


Diamo a Cesare ciò che è di Cesare

public class Location { int x = 0; int y = 0; public Location (int x, int y) { this.x = x; this.y = y; }

}


bird.setLocation(location);if (!(bird instanceof Chicken)) bird.setHeight(new Random().nextInt(this.depth));




Bird duck = new Duck();duck.setLocation(new Location(10,5));duck.setHeight(3);




Bird duck = new Duck();duck.setLocation(new Location(10,5));duck.setHeight(3);

int h = bird.getHeight();Location location = bird.getLocation();int x = location.x;int y = location.y;isValid = fieldSize.isWithinField(h, x, y);



public class Location { int x = 0; int y = 0;

int h = 0;

public Location (int x, int y) { this(x, y, 0); }

public Location (int x, int y, int z) { this.x = x; this.y = y; this.h = z; }

}



Location location = new Location(new Random().nextInt(fieldSize.width()), new Random().nextInt(fieldSize.height()), new Random().nextInt(fieldSize.depth()));



public abstract class Bird { Location location; int height; public void setHeight(int height) throws Exception{ this.location.h = height; } public int getHeight() { return location.h; } public void setLocation(Location location) { this.location = location; } public Location getLocation() { return location; } public abstract void sing();}



private boolean isGameFieldValid(){ boolean isValid = true; for(Bird bird : birds) { int h = bird.getHeight(); Location location = bird.getLocation(); int x = location.x; int y = location.y; isValid = fieldSize.isWithinField(h, x, y); if (!isValid) break; } return isValid;}



private boolean isGameFieldValid(){ boolean isValid = true; for(Bird bird : birds) { Location location = bird.getLocation(); int h = location.h; int x = location.x; int y = location.y; isValid = fieldSize.isWithinField(h, x, y); if (!isValid) break; } return isValid;}



public boolean shot(int x, int y, int h) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { int height = bird.getHeight(); Location location = bird.getLocation(); hit = location.x == x && location.y == y && height == h; if (hit) { bird.sing(); break; } } } return hit; }



public boolean shot(int x, int y, int h) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { Location location = bird.getLocation(); hit = location.x == x && location.y == y && location.h == h; if (hit) { bird.sing(); break; } } } return hit; }



Quindi ora?public abstract class Bird { Location location; int height; public void setHeight(int height) throws Exception{ this.height = height; } public int getHeight() { return height; } public void setLocation(Location location) { this.location = location; } public Location getLocation() { return location; } public abstract void sing();}


Tell, don’t Askpublic boolean shot(int x, int y, int h) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { Location location = bird.getLocation(); hit = location.x == x && location.y == y && location.h == h; if (hit) { bird.sing(); break; } } } return hit; }


Applied

public boolean shot(Location shotLocation) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { hit = shotLocation.equals(bird.getLocation()); if (hit) { bird.sing(); break; } } } return hit; }


Let’s apply it another time

public boolean shot(Location shotLocation) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { hit = bird.wasHit(shotLocation) if (hit) { bird.sing(); break; } } } return hit; }


Another time too

public boolean shot(Location shotLocation) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { hit = bird.wasHit(shotLocation) if (hit) { break; } } } return hit; }


Programmiamo ad oggetti o a tipi primitivi?

private boolean isGameFieldValid(){ boolean isValid = true; for(Bird bird : birds) { Location location = bird.getLocation(); int h = location.h; int x = location.x; int y = location.y; isValid = fieldSize.isWithinField(h, x, y); if (!isValid) break; } return isValid;}


e allora si fa!

private boolean isGameFieldValid(){ boolean isValid = true; for(Bird bird : birds) { isValid = fieldSize.isWithinField(bird.getLocation()); if (!isValid) break; } return isValid;}


Serve ancora?

private void placeBirds(PlacingMode type) throws Exception { //Random Distribution if (type == PlacingMode.Random) { for(Bird bird : birds) { Location location = new Location(new Random().nextInt(fieldSize.width()), new Random().nextInt(fieldSize.eighth()), new Random().nextInt(fieldSize.depth())); bird.setLocation(location); if (!(bird instanceof Chicken)) bird.setHeight(new Random().nextInt(this.depth)); } } //Custom Distribution else if (type == PlacingMode.Custom) { } }


E allora rimuoviamolo

private void placeBirds(PlacingMode type) throws Exception { //Random Distribution if (type == PlacingMode.Random) { for(Bird bird : birds) { Location location = new Location(new Random().nextInt(fieldSize.width()), new Random().nextInt(fieldSize.eighth()), new Random().nextInt(fieldSize.depth())); bird.setLocation(location); } } //Custom Distribution else if (type == PlacingMode.Custom) { } }


Anche il metodo

public abstract class Bird { ... public void setHeight(int height) throws Exception{ this.height = height; }

...}


Gli IF proprio non ci piacciono!


Come procediamo?

private void placeBirds(PlacingMode type) throws Exception { //Random Distribution if (type == PlacingMode.Random) { for(Bird bird : birds) { Location location = new Location(new Random().nextInt(fieldSize.width()), new Random().nextInt(fieldSize.eighth()), new Random().nextInt(fieldSize.depth())); bird.setLocation(location); } } //Custom Distribution else if (type == PlacingMode.Custom) { } }


I Passi di refactoring• Estratta responsabilità di “Random placing strategy” in

una classe

• estratta interfaccia IPlacingStrategy

• creata class NullPlacingStrategy

• creata Factory per Placing strategy

• “inlainato” metodo placeBirds

• trasformata la factory in un field ed estratto come parametro del costruttore


Il risultatopublic interface IPlacingStrategy {

void place(List<Bird> birds);

}

public class NullPlacingStrategy implements IPlacingStrategy {

@Override public void place(List<Bird> birds) { // Do nothing }

}

public class RandomPlacingStrategy implements IPlacingStrategy { private FieldSize fieldSize;

public RandomPlacingStrategy(FieldSize fieldSize) { this.fieldSize = fieldSize; }

@Override public void place(List<Bird> birds) { for(Bird bird : birds) { Location location = new Location(new Random().nextInt(fieldSize.width()), new Random().nextInt(fieldSize.height()), new Random().nextInt(fieldSize.depth())); bird.setLocation(location); } }

}

public class PlacingStrategyFactory {

public IPlacingStrategy create(PlacingMode type, FieldSize fieldSize) { if (type == PlacingMode.Random) { return new RandomPlacingStrategy(fieldSize); } return new NullPlacingStrategy(); }

}


Il risultato

public class GameField {...

public boolean startGame(PlacingMode pm) { placingStrategyFactory.create(pm, fieldSize).place(birds); gameStarted = isGameStarted(); return gameStarted; }

...}


Altri IF che non ci piacciono

private boolean isGameFieldValid(){ boolean isValid = true; for(Bird bird : birds) { isValid = fieldSize.isWithinField(bird.getLocation()); if (!isValid) break; } return isValid;}


Così è meglio

private boolean isGameFieldValid(){ boolean isValid = true; for(Bird bird : birds) { isValid = isValid && fieldSize.isWithinField(bird.getLocation()); } return isValid;}


Stesso discorso

public boolean shot(Location shotLocation) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { hit = bird.wasHit(shotLocation) if (hit) { break; } } } return hit; }


Diventa

public boolean shot(Location shotLocation) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { hit = hit || bird.wasHit(shotLocation) } } return hit; }


Di nuovo sulle responsabilità


Notate qualcosa?private boolean isGameFieldValid(){ boolean isValid = true; for(Bird bird : birds) { isValid = isValid && fieldSize.isWithinField(bird.getLocation()); } return isValid;}

public boolean shot(Location shotLocation) { boolean hit = false; if (gameStarted) { for(Bird bird : birds) { hit = hit || bird.wasHit(shotLocation) } } return hit; }


BirdListpublic class BirdList extends ArrayList<Bird> { private static final long serialVersionUID = -3323859086260693300L; public boolean areAllBirdsPlacedWithinField(FieldSize fieldSize) { boolean isValid = true; for(Bird bird : this) { isValid = isValid && fieldSize.isWithinField(bird.getLocation()); } return size() > 0 && isValid;

}

public boolean anyBirdWasHit(Location shotLocation) { boolean hit=false; for(Bird bird : this) { hit = hit || bird.wasHit(shotLocation); } return hit; }}


ed ecco i chiamanti

private boolean isGameStarted() { return birds.areAllBirdsPlacedWithinField(fieldSize);}public boolean shot(Location shotLocation) { return birds.anyBirdWasHit(shotLocation) && gameStarted; }


I Repository





GRAZIE

Antonio Carpentieri (@acarpe)Stefano Leli (@sleli)


Codice legacy, usciamo dal pantano!

Sports

Transcript of Codice legacy, usciamo dal pantano!