Zwei überladene Methoden, gleiche Aktionen, doppelter Code

Question

Ich habe zwei Methoden, die entweder eine Circle oder eine Scene nehmen und den Hintergrund rot - grün - blau blinken lassen, indem Sie den Wert fill alle N ms ändern.

Die beiden Methoden:

private void makeRGB(Circle c) { 
    Timer t = new Timer(); 
    t.scheduleAtFixedRate(new TimerTask() { 
     @Override 
     public void run() { 
      if(c.getFill() == Color.RED) { 
       c.setFill(Color.GREEN); 
      } else if (c.getFill() == Color.GREEN) { 
       c.setFill(Color.BLUE); 
      } else { 
       c.setFill(Color.RED); 
      } 
     } 
    },0, RGB_CHANGE_PERIOD); 
} 

private void makeRGB(Scene s) { 
    Timer t = new Timer(); 
    t.scheduleAtFixedRate(new TimerTask() { 
     @Override 
     public void run() { 
      if(s.getFill() == Color.RED) { 
       s.setFill(Color.GREEN); 
      } else if (s.getFill() == Color.GREEN) { 
       s.setFill(Color.BLUE); 
      } else { 
       s.setFill(Color.RED); 
      } 
     } 
    },0, RGB_CHANGE_PERIOD); 
} 

Offensichtlich sind diese sehr ähnlich, aber wie Circle und Scene nicht in der gleichen Vererbungsbaum sind ich nicht den Ansatz, den Aufruf einer übergeordneten Klasse von ihnen die beide enthalten, verwenden können .setFill()/.getFill() Methoden.

Wie würde ich Code-Duplizierung hier entfernen?

Answer 1

Im Allgemeinen entfernen Sie doppelten Code, indem Sie den allgemeinen Code in eine Funktion/Methode/Klasse einbeziehen und die Teile, die variieren, parametrisieren. In diesem Fall variiert die Art, wie Sie die aktuelle Füllung abrufen und wie Sie die neue Füllung festlegen. Das java.util.function Paket enthält die entsprechenden Typen für Parametrisierung diese, so können Sie dies tun:

private void makeRGB(Circle c) { 
    makeRGB(c::getFill, c:setFill); 
} 

private void makeRGB(Scene s) { 
    makeRGB(s::getFill, s:setFill); 
} 

private void makeRGB(Supplier<Paint> currentFill, Consumer<Paint> updater) { 
    Timer t = new Timer(); 
    t.scheduleAtFixedRate(new TimerTask() { 
     @Override 
     public void run() { 
      if(currentFill.get() == Color.RED) { 
       updater.accept(Color.GREEN); 
      } else if (currentFill.get() == Color.GREEN) { 
       updater.accept(Color.BLUE); 
      } else { 
       updater.accept(Color.RED); 
      } 
     } 
    },0, RGB_CHANGE_PERIOD); 
} 

Beachten Sie jedoch, dass Sie nicht die Benutzeroberfläche von einem Hintergrundthread ändern sollte. Sie sollten sich wirklich

private void makeRGB(Supplier<Paint> currentFill, Consumer<Paint> updater) { 
    Timer t = new Timer(); 
    t.scheduleAtFixedRate(new TimerTask() { 
     @Override 
     public void run() { 
      Platform.runLater(() -> { 
       if(currentFill.get() == Color.RED) { 
        updater.accept(Color.GREEN); 
       } else if (currentFill.get() == Color.GREEN) { 
        updater.accept(Color.BLUE); 
       } else { 
        updater.accept(Color.RED); 
       } 
      } 
     } 
    },0, RGB_CHANGE_PERIOD); 
} 

oder (besser) tun, use a Timeline Dinge in regelmäßigen Abständen zu tun.

Wie in den Kommentaren erwähnt, können Sie auch eine Map bereitstellen, die jede Farbe der Farbe nachordnet, die danach kommt. Die Kombination all dies gibt:

private final Map<Paint, Paint> fills = new HashMap<>(); 

// ... 

    fills.put(Color.RED, Color.GREEN); 
    fills.put(Color.GREEN, Color.BLUE); 
    fills.put(Color.BLUE, Color.RED); 

// ... 

private void makeRGB(Circle c) { 
    makeRGB(c::getFill, c:setFill); 
} 

private void makeRGB(Scene s) { 
    makeRGB(s::getFill, s:setFill); 
} 

private void makeRGB(Supplier<Paint> currentFill, Consumer<Paint> updater) { 

    Timeline timeline = new Timeline(Duration.millis(RGB_CHANGE_PERIOD), 
     e-> updater.accept(fills.get(currentFill.get()))); 
    timeline.setCycleCount(Animation.INDEFINITE); 
    timeline.play(); 
}