2.4. Clean Code: Kein doppelter Code!
Eines der wichtigsten Refactorings in der Softwareentwicklung ist die Eliminierung von doppeltem Code. Dies wird um so einfacher, je mehr sich die Codestellen gleichen und je weniger Elemente von außen genutzt werdem. Im einfachsten Fall kann man eine neue Methode erstellen, in die der doppelte Code kopiert wird. Die Elemente außerhalb des Codes werden zu Parametern und ein Ergebnis wird von der Methode zurückgegeben.
Wenn der Code in unterschiedlichen Klassen ist, mehr wie ein Ergebnis entsteht oder sehr viele Eingaben von außen erfolgen müssen, dann ist eine eigenständige Klasse notwendig.
Um etwas umstellen zu können, ist als Erstes immer das Erkennen von doppeltem Code notwendig. Beim Schreiben der Klassen ImageService und LevelService ist vielleicht schon aufgefallen, dass sich diese sehr ähnlich sind:
- Der Name XxxService
- Die Konstanten XXX_RESOURCE_TYPE und XXX_RESOURCE_PATH
- Die Methode xxxLoad(String) welche ein Objekt von Typ Xxx zurück gibt.
- Der Aufbau der Methode - es wird erst ein Pfad konstruiert, die Ressource wird über einen InputStream gelesen. Nur die Verarbeitung der Daten ist unterschiedlich.
Wenn wir erkannt haben, dass die Klassen sich sehr ähneln, können wir uns überlegen, wie dies vereinheitlicht werden kann.
Ein Problem ist, dass die load-Methoden unterschiedliche Rückgabetypen haben. Da wir nicht wissen können, was für eine Ressource wir laden, können wir hier nur einen Platzhalter angeben: ein Generic. Diese werden in der Regel durch einen großen Buchstaben angegeben und wir könnten hier R wählen (R wie “Rückgabe”).
Mit dieser Überlegung können wir uns schon einmal den Rahmen der Klasse bzw. der Methode bauen: Einen universellen ResourceServer, der bei loadResource ein Element vom Typ R zurückgibt:
package org.jadv.services;
public class ResourceService<R> {
protected R loadResource(String resourceName) {
return null;
}
}
Wir sind natürlich noch lange nicht fertig, aber die bestehenden Klassen könnten darauf schon zugreifen. Dies würde dann zu folgenden Anpassungen führen:
public class LevelService extends ResourceService<Level> {
// ...
public Level loadLevel(String resourceName) {
return loadResource(resourceName);
}
// ...
}
Bei den Klassen war ein weiterer Unterschied, dass für den Präfix und den Typ der Ressource jeweils zwei Konstanten bereitgestellt wurden. Diese Konstanten benötigen wir dann auch in unserer neuen Klasse. Jedoch können wir hier die Werte noch nicht wissen. Daher müssen die Werte dieser Konstanten nicht über Literale gesetzt werden sondern durch den Konstruktor, so dass es keine Konstanten im bisherigen Sinne sind, sondern finale Instanzvariablen.
Im Code ergänzt würde dies bedeuten:
package org.jadv.services;
public class ResourceService<R> {
private final String prefix;
private final String type;
public ResourceService(String prefix, String type) {
this.prefix = prefix;
this.type = type;
}
public R loadResource(String resourceName) {
return null;
}
}
In den Klassen, die diesen ResourceService nutzen, müssen dann im Konstruktor diese Werte angegeben werden:
public class LevelService extends ResourceService<Level> {
private static final String LEVEL_RESOURCE_TYPE = ".json";
private static final String LEVEL_RESOURCE_PATH = "/level/";
public LevelService() {
super(LEVEL_RESOURCE_PATH, LEVEL_RESOURCE_TYPE);
}
// ...
}
Nun müssen wir natürlich noch die Methode loadResource füllen. Bei Betrachtung der Methode loadLevel sehen wir, dass vieles schon passt:
- Es wird ein Pfad für die Ressource erstellt
- Es wird ein InputStream erstellt und geprüft.
Lediglich der Code, der aus dem InputStream eine Instanz von Level macht, ist unterschiedlich.
Übernehmen wir erst einmal den Code, der 1:1 übernommen werden kann:
public R loadResource(String resourceName) {
String resourcePath = prefix + resourceName + type;
try (InputStream inputStream = getClass().getResourceAsStream(resourcePath)) {
if (inputStream == null) {
return null;
}
return null; // TODO: Was kann man hier machen?
} catch (IOException exception) {
System.out.println("Unable to load level resource: " + resourcePath + " (" + exception.getMessage() + ")");
exception.printStackTrace();
}
return null;
}
Bei der Zeile TODO müssen wir etwas verändern, denn wir brauchen da irgendeine Funktion, die einen InputStream nimmt und dann ein “R” zurückgibt.
Und mit dieser Formulierung haben wir schon die Lösung gefunden: Wir brauchen eine Funktion dafür. Und diese Funktion kann von außen übergeben werden. Wir können der Methode so eine Funktion übergeben, z.B. per: java.util.function.Function<T,R>.
Wenn wir dies in die Methode einbauen, dann erhalten wir:
public R loadResource(String resourceName, Function<InputStream, R> function) {
String resourcePath = prefix + resourceName + type;
try (InputStream inputStream = getClass().getResourceAsStream(resourcePath)) {
if (inputStream == null) {
return null;
}
return function.apply(inputStream);
} catch (IOException exception) {
System.out.println("Unable to load level resource: " + resourcePath + " (" + exception.getMessage() + ")");
exception.printStackTrace();
}
return null;
}
Damit würde die Klasse ResourceService am Ende so aussehen:
package org.jadv.services;
import java.io.IOException;
import java.io.InputStream;
import java.util.function.Function;
public class ResourceService<R> {
private final String prefix;
private final String type;
public ResourceService(String prefix, String type) {
this.prefix = prefix;
this.type = type;
}
public R loadResource(String resourceName, Function<InputStream, R> function) {
String resourcePath = prefix + resourceName + type;
try (InputStream inputStream = getClass().getResourceAsStream(resourcePath)) {
if (inputStream == null) {
return null;
}
return function.apply(inputStream);
} catch (IOException exception) {
System.out.println("Unable to load level resource: " + resourcePath + " (" + exception.getMessage() + ")");
exception.printStackTrace();
}
return null;
}
}
Da bei dem vorhandenen Interface Function<T,R> bei der Methode keine Exception geworfen werden kann, macht es bei so einer Lösung Sinn, ein eigenes Interface zu erzeugen. Im Code könnte dies so aussehen:
public abstract class ResourceService<R> {
public interface ResourceCreator<R> {
R create(InputStream inputStream);
}
public R loadResource(String resourceName, ResourceCreator<R> creator) {
String resourcePath = prefix + resourceName + type;
try (InputStream inputStream = getClass().getResourceAsStream(resourcePath)) {
if (inputStream == null) {
return null;
}
return creator.create(inputStream);
} catch (IOException exception) {
System.out.println("Unable to load level resource: " + resourcePath + " (" + exception.getMessage() + ")");
exception.printStackTrace();
}
return null;
}
Damit haben wir nun eine Basisklasse, die wir in unseren bisherigen Services nutzen können:
package org.jadv.services;
import com.google.gson.Gson;
import org.jadv.model.SavedObject;
import org.jadv.model.level.Level;
import java.io.InputStream;
import java.io.InputStreamReader;
/**
* Service to load a Level of a resource.
*/
public class LevelService extends ResourceService<Level> {
/**
* type of level resources.
*/
private static final String LEVEL_RESOURCE_TYPE = ".json";
/**
* Resource path of level files.
*/
private static final String LEVEL_RESOURCE_PATH = "/level/";
/**
* Creates a new instance of SavedObjectService.
*/
public LevelService() {
super(LEVEL_RESOURCE_PATH, LEVEL_RESOURCE_TYPE);
}
/**
* Loads a Level of an InputStream
* @param inputStream InputStream to load Level from.
* @return Loaded level.
*/
protected Level loadLevel(InputStream inputStream) {
Gson gson = new Gson();
return (Level) gson.fromJson(new InputStreamReader(inputStream), SavedObject.class);
}
/**
* Loads a Level of a given resource name.
*
* @param resourceName Name of the resource.
* @return The level if resource was found, else null.
*/
public Level loadLevel(String resourceName) {
return loadResource(resourceName, this::loadLevel);
}
}
Wir hatten festgestellt, dass die Methode loadRessource eine Funktion benötigt, die aus einem InputStream ein Element vom Typ R macht. Statt diese als Parameter zu übergeben ist es auch möglich, so eine Methode in der Klasse als abstrakte Methode vorzugeben. Da wir in der Methode IOExceptions fangen, können wir in dieser Methode auch das Werfen von IOException erlauben. Eine Lösung könnte so aussehen:
protected abstract R createResource(InputStream inputStream) throws IOException;
public R loadResource(String resourceName) {
String resourcePath = prefix + resourceName + type;
try (InputStream inputStream = getClass().getResourceAsStream(resourcePath)) {
if (inputStream == null) {
return null;
}
return createResource(inputStream);
} catch (IOException exception) {
System.out.println("Unable to load level resource: " + resourcePath + " (" + exception.getMessage() + ")");
exception.printStackTrace();
}
return null;
}
Die Verwendung ändert sich dann in LoadLevel zu:
public Level loadLevel(String resourceName) {
return loadResource(resourceName);
}
@Override
protected Level createResource(InputStream inputStream) {
Gson gson = new Gson();
return (Level) gson.fromJson(new InputStreamReader(inputStream), SavedObject.class);
}
Welche Lösung bevorzugt wird, darf jeder für sich entscheiden. Ich sehe für beide Varianten Vor- und Nachteile:
-
Bei der Version mit der Function als zusätzlicher Parameter hat man den Vorteil, dass der Name der Methode bei der Implementation frei wählbar ist. Der Name kann also genau aussagen, was dort in der Methode gemacht wird. Dadurch sind die ablgeleiteten Klassen einfacher zu lesen.
-
Bei der zweiten Version mit der abstrakten Methode ist der Name der Methode vorgegeben: Die Methode muss createResource heissen. Damit kann man nicht mehr den Namen so wählen, wie er aus Sicht der abgeleiteten Klasse am besten wäre. Und da wir eine IOException erlauben können, muss hier keine zusätzliche Behandlung in der Methode selbst erfolgen.
Wir werden im weiteren Verlauf die letztere Version verwenden. Es sollte aber im weiteren Verlauf unkritisch sein - wenn Du die andere Variante bevorzugst, dann kannst Du in Deinem Code auch gerne die andere Variante weiter verwenden.