vendredi 14 août 2015

Observer Pattern - bad smell in code

I'm doing some work to understand design patterns in more depth. I wrote a bit of sample code for the Observer pattern, but there is something about it that doesn't smell right, and I would be interested in other's opinions of if/where this is off track. I have 3 classes. The Observable implementation:

package observer;

import java.util.HashMap;
import java.util.Observable;
import java.util.Observer;

public class ScoreData extends Observable {

   public HashMap<String, String> scores = new HashMap<String, String> ();

   public ScoreData() { }

   public void scoresChanged() {
     setChanged();
     notifyObservers();
   }

   public void setScores(String sport, String score) {
     scores.put(sport, score);
     scoresChanged();
   }

   public String getScore(String sport) {
     return (String) scores.get(sport);
   }    

}

Then the Observer implementation:

package observer;

import java.util.Observable;
import java.util.Observer;

public class SportsFan implements Observer {
    private String currentResult;  
    private String previousGame;
    private String sport;

    public SportsFan(Observable observable, String sport) {
        this.sport = sport;
        observable.addObserver(this);   
    }

    public void update(Observable observable, Object arg) {
            ScoreData scoreData = (ScoreData)observable;
            previousGame = currentResult;
            currentResult = scoreData.getScore(sport);
            reportScores();
    }

    public void reportScores() {        
            if (this.currentResult != this.previousGame)
            System.out.println("Results of the " + sport + "  Game: " + currentResult);
    }   

}

And finally my test harness:

package observer;

public class Sportscaster {

    public static void main(String[] args) {

            ScoreData scores = new ScoreData();

            SportsFan hockeyFan = new SportsFan(scores, "Hockey");
            SportsFan baseballFan = new SportsFan(scores, "Baseball");
            SportsFan footballFan =  new SportsFan(scores, "Football");
            scores.setScores("Hockey", "Vancouver 2, Winnepeg 0");
            scores.setScores("Baseball", "Brewers 7, Cardinals 2");
            scores.setScores("Football", "Raiders 21, Patriots 13");
            scores.setScores("Football", "Vikings 34, Packers 17");
            scores.setScores("Baseball", "Astros 10, Royals 7");

    }

}

When I run this, I get the following:

Results of the Hockey  Game: Vancouver 2, Winnepeg 0
Results of the Baseball  Game: Brewers 7, Cardinals 2
Results of the Football  Game: Raiders 21, Patriots 13
Results of the Football  Game: Vikings 34, Packers 17
Results of the Baseball  Game: Astros 10, Royals 7

This is fine - works well. But the part of the code that has a bad smell to me is where I qualify the output:

if (this.currentResult != this.previousGame)
            System.out.println("Results of the " + sport + "  Game: " + currentResult);

If I remove the if statement, I of course get:

Results of the Football  Game: null
Results of the Baseball  Game: null
Results of the Hockey  Game: Vancouver 2, Winnepeg 0
Results of the Football  Game: null
Results of the Baseball  Game: Cubs 7, Cardinals 2
Results of the Hockey  Game: Vancouver 2, Winnepeg 0
Results of the Football  Game: Raiders 21, Patriots 13
Results of the Baseball  Game: Cubs 7, Cardinals 2
Results of the Hockey  Game: Vancouver 2, Winnepeg 0
Results of the Football  Game: Bears 34, Packers 17
Results of the Baseball  Game: Cubs 7, Cardinals 2
Results of the Hockey  Game: Vancouver 2, Winnepeg 0
Results of the Football  Game: Bears 34, Packers 17
Results of the Baseball  Game: White Sox 10, Royals 7
Results of the Hockey  Game: Vancouver 2, Winnepeg 0

Is there a better/cleaner way of doing this?

Aucun commentaire:

Enregistrer un commentaire