Gute oder schlechte Praxis, Java-Sammlungen mit aussagekräftigen Klassennamen zu maskieren?

46

In letzter Zeit habe ich es mir zur Gewohnheit gemacht, Java-Sammlungen mit menschlichen Klassennamen zu "maskieren". Einige einfache Beispiele:

// Facade class that makes code more readable and understandable.
public class WidgetCache extends Map<String, Widget> {
}

Oder:

// If you saw a ArrayList<ArrayList<?>> being passed around in the code, would you
// run away screaming, or would you actually understand what it is and what
// it represents?
public class Changelist extends ArrayList<ArrayList<SomePOJO>> {
}

Ein Kollege wies mich darauf hin, dass dies eine schlechte Praxis ist, und führte Verzögerung / Latenz ein sowie ein OO-Antimuster. Ich kann es verstehen, was einen sehr geringen Leistungsaufwand bedeutet, aber ich kann mir nicht vorstellen, dass es überhaupt bedeutsam ist. Also frage ich: ist das gut oder schlecht und warum?

Herpylderp
quelle
11
Es ist viel einfacher als das. Es ist eine schlechte Praxis, weil ich mir vorstelle, dass Sie die Implementierungen dieser Java Basic-JDK-Sammlung erweitern. In Java können Sie nur eine Klasse erweitern, sodass Sie mit einer Erweiterung mehr überlegen und gestalten müssen. Verwenden Sie in Java die Option "Erweitern" sparsam.
InformedA
ChangeListKompilierung wird an brechen extends, weil Liste eine Schnittstelle ist, erfordert implements. @ RandomA, was Sie sich vorstellen, verfehlt den Punkt wegen dieses Fehlers
Gnat
@gnat Es fehlt nicht der Punkt, ich gehe davon aus, dass er ein implementationie erweitert hat HashMapoder TreeMapund was er dort hatte, war ein Tippfehler.
Informiert am
4
Dies ist eine schlechte Praxis. SCHLECHT SCHLECHT SCHLECHT. Mach das nicht. Jeder weiß, was eine Map <String, Widget> ist. Aber ein WidgetCache? Jetzt muss ich WidgetCache.java öffnen und daran denken, dass WidgetCache nur eine Karte ist. Ich muss jedes Mal überprüfen, wenn eine neue Version herauskommt, dass Sie WidgetCache nicht neu hinzugefügt haben. Gott nein, tu das niemals.
Miles Rout
"Wenn Sie eine ArrayList <ArrayList <? >> sehen würden, die im Code herumgereicht wird, würden Sie schreiend davonlaufen ..." Nein, ich bin ziemlich zufrieden mit verschachtelten generischen Sammlungen. Und du solltest es auch sein.
Kevin Krumwiede

Antworten:

75

Verzögerung / Latenz? Darauf rufe ich BS an. Diese Praxis sollte einen Overhead von genau Null verursachen. ( Bearbeiten: In den Kommentaren wurde darauf hingewiesen, dass dies tatsächlich die von der HotSpot-VM durchgeführten Optimierungen verhindern kann. Ich weiß nicht genug über die VM-Implementierung, um dies zu bestätigen oder abzulehnen. Ich habe meinen Kommentar auf C ++ gestützt Implementierung virtueller Funktionen.)

Es gibt einige Code-Overhead. Sie müssen alle Konstruktoren aus der gewünschten Basisklasse erstellen und ihre Parameter weiterleiten.

Ich sehe es auch nicht als Anti-Muster an sich. Ich sehe es jedoch als verpasste Gelegenheit. Anstatt eine Klasse zu erstellen, die die Basisklasse nur zum Umbenennen ableitet, können Sie auch eine Klasse erstellen, die die Auflistung enthält und eine fallspezifische, verbesserte Schnittstelle bietet. Sollte Ihr Widget-Cache wirklich die vollständige Oberfläche einer Karte bieten? Oder sollte es stattdessen eine spezielle Schnittstelle bieten?

Darüber hinaus funktioniert das Muster bei Auflistungen einfach nicht mit der allgemeinen Regel der Verwendung von Schnittstellen und nicht mit Implementierungen zusammen. Im einfachen Auflistungscode würden Sie eine erstellen HashMap<String, Widget>und sie dann einer Variablen vom Typ zuweisen Map<String, Widget>. Sie WidgetCachekönnen nicht erweitern Map<String, Widget>, da dies eine Schnittstelle ist. Es kann keine Schnittstelle sein, die die Basisschnittstelle erweitert, da HashMap<String, Widget>diese Schnittstelle nicht implementiert wird und auch keine andere Standardauflistung. Und während Sie eine Klasse erstellen können, die erweitert werden soll HashMap<String, Widget>, müssen Sie die Variablen als WidgetCacheoder deklarieren Map<String, Widget>, und die erste verliert die Flexibilität, eine andere Sammlung zu ersetzen (möglicherweise die verzögerte Ladesammlung einiger ORMs), während die zweite Art den Punkt besiegt die Klasse zu haben.

Einige dieser Kontrapunkte gelten auch für meine vorgeschlagene Fachklasse.

Dies sind alles Punkte, die zu beachten sind. Es kann die richtige Wahl sein oder auch nicht. In beiden Fällen sind die von Ihrem Kollegen angebotenen Argumente nicht gültig. Wenn er denkt, dass es ein Anti-Muster ist, sollte er es benennen.

Sebastian Redl
quelle
1
Beachten Sie jedoch, dass es zwar durchaus zu Leistungseinbußen kommen kann, aber nicht so schlimm sein wird (im schlimmsten Fall fehlen einige Inlining-Optimierungen und ein zusätzlicher Anruf - nicht besonders in der innersten Schleife, aber ansonsten wahrscheinlich in Ordnung).
Voo
5
Diese wirklich gute Antwort sagt jedem: "Beurteile die Entscheidung nicht anhand des Leistungsaufwands" - und die einzige Diskussion hier unten ist: "Wie geht HotSpot damit um, gibt es einige (in 99,9% aller Fälle) irrelevante Auswirkungen auf die Leistung?" Hast du dir überhaupt die Mühe gemacht, mehr als den ersten Satz zu lesen?
Doc Brown
16
+1 für "Anstatt eine Klasse zu erstellen, die die Basisklasse nur zum Umbenennen ableitet, erstellen Sie stattdessen eine Klasse, die die Auflistung enthält und eine fallspezifische, verbesserte Schnittstelle bietet?"
user11153
6
Ein praktisches Beispiel, das den Hauptpunkt dieser Antwort veranschaulicht: In der Dokumentation für public class Properties extends Hashtable<Object,Object>in java.utilheißt es: "Da Properties von Hashtable erbt, können die put- und putAll-Methoden auf ein Properties-Objekt angewendet werden. Von ihrer Verwendung wird dringend abgeraten, da sie dem Aufrufer das Einfügen von Einträgen ermöglichen, deren Schlüssel oder Werte sind keine Strings. " Die Zusammensetzung wäre viel sauberer gewesen.
Patricia Shanahan
3
@DocBrown Nein, diese Antwort gibt an, dass die Leistung überhaupt nicht beeinträchtigt wird. Wenn du starke Aussagen machst, kannst du sie besser unterstützen, sonst werden dich die Leute wahrscheinlich darauf aufmerksam machen. Der Sinn von Kommentaren (zu Antworten) besteht darin, ungenaue Fakten oder Annahmen herauszustellen oder nützliche Notizen hinzuzufügen, aber sicherlich nicht den Menschen zu gratulieren. Dafür ist das Abstimmungssystem da.
Voo
25

Laut IBM handelt es sich tatsächlich um ein Anti-Pattern. Diese 'typedef'-ähnlichen Klassen werden Pseudotypen genannt.

Der Artikel erklärt es viel besser als ich, aber ich werde versuchen, es zusammenzufassen, falls der Link ausfällt:

  • Jeder Code, der a erwartet, WidgetCachekann a nicht verarbeitenMap<String, Widget>
  • Diese Pseudotypen sind "viral", wenn sie mehrere Pakete verwenden. Sie führen zu Inkompatibilitäten, während der Basistyp (nur eine alberne Map <...>) in allen Fällen in allen Paketen funktioniert hätte.
  • Pseudotypen sind oft zu konkret, sie implementieren keine spezifischen Schnittstellen, da ihre Basisklassen nur die generische Version implementieren.

In dem Artikel schlagen sie den folgenden Trick vor, um das Leben ohne Verwendung von Pseudotypen zu vereinfachen:

public static <K,V> Map<K,V> newHashMap() {
    return new HashMap<K,V>(); 
}

Map<Socket, Future<String>> socketOwner = Util.newHashMap();

Was aufgrund der automatischen Typinferenz funktioniert.

(Ich bin zu dieser Antwort über diese verwandte Stapelüberlauf-Frage gekommen.)

Roy T.
quelle
13
Wird der Bedarf newHashMapjetzt nicht vom Diamantenbetreiber gelöst?
Svick
Absolut wahr, habe das vergessen. Normalerweise arbeite ich nicht in Java.
Roy T.
Ich wünschte, die Sprachen würden ein Universum von Variablentypen zulassen, die Verweise auf Instanzen anderer Typen enthielten, aber dennoch die Überprüfung zur Kompilierungszeit unterstützen könnten, sodass ein Wert vom Typ WidgetCacheeiner Variablen vom Typ WidgetCacheoder Map<String,Widget>ohne Umwandlung zugewiesen werden könnte , aber dort war eine Methode, mit der eine statische Methode WidgetCacheeinen Verweis auf einen Map<String,Widget>Typ WidgetCacheerstellen und ihn als Typ zurückgeben konnte, nachdem eine gewünschte Validierung durchgeführt wurde. Eine solche Funktion könnte besonders bei Generika nützlich sein, die nicht vom Typ gelöscht werden müssten (seit ...
supercat 16.03.15
... die Typen würden sowieso nur im Kopf des Compilers existieren). Für viele veränderbare Typen gibt es zwei gebräuchliche Typen von Referenzfeldern: einen, der die einzige Referenz auf eine Instanz enthält, die mutiert werden kann, oder einen, der eine gemeinsam nutzbare Referenz auf eine Instanz enthält, die niemand mutieren darf. Es wäre hilfreich, diesen beiden Arten von Feldern unterschiedliche Namen zu geben, da sie unterschiedliche Verwendungsmuster erfordern, aber beide Verweise auf dieselben Arten von Objektinstanzen enthalten sollten.
Supercat
5
Dies ist ein gutes Argument dafür, warum Java Typ-Aliase benötigt.
GlenPeterson
13

Der Leistungseinbruch würde sich höchstens auf eine Vtable-Suche beschränken, die Sie höchstwahrscheinlich bereits durchführen. Das ist kein triftiger Grund, sich dagegen zu wehren.

Die Situation ist häufig genug, dass die meisten statisch typisierten Programmiersprachen eine spezielle Syntax für Aliasing-Typen haben, die normalerweise als a bezeichnet wird typedef. Java hat diese wahrscheinlich nicht kopiert, da es ursprünglich keine parametrisierten Typen gab. Eine Klassenerweiterung ist aus den Gründen, die Sebastian in seiner Antwort so ausführlich dargelegt hat, nicht ideal, kann aber eine vernünftige Lösung für Javas eingeschränkte Syntax sein.

Typedefs haben eine Reihe von Vorteilen. Sie drücken die Absicht des Programmierers mit einem besseren Namen auf einer angemesseneren Abstraktionsebene deutlicher aus. Sie sind für Debugging- oder Refactoring-Zwecke einfacher zu suchen. Überlegen Sie, wo immer a verwendet WidgetCachewird, und suchen Sie nach den spezifischen Verwendungszwecken von a Map. Sie sind leichter zu ändern, zum Beispiel , wenn Sie später finden Sie eine LinkedHashMapStelle, oder sogar Ihre eigenen Container.

Karl Bielefeldt
quelle
Es gibt auch einen winzigen Aufwand in den Konstruktoren.
Stephen C
11

Ich empfehle Ihnen, wie bereits erwähnt, Komposition anstelle von Vererbung zu verwenden, damit Sie nur die wirklich benötigten Methoden mit Namen verfügbar machen können, die dem beabsichtigten Anwendungsfall entsprechen. Müssen Benutzer Ihrer Klasse wirklich wissen, dass WidgetCachees sich um eine Karte handelt? Und damit machen können, was sie wollen? Oder müssen sie nur wissen, dass dies ein Cache für Widgets ist?

Beispielklasse aus meiner Codebasis mit Lösung für ein ähnliches Problem, das Sie beschrieben haben:

public class Translations {

    private Map<Locale, Properties> translations = new HashMap<>();

    public void appendMessage(Locale locale, String code, String message) {
        /* code */
    }

    public void addMessages(Locale locale, Properties messages) {
        /* code */
    }

    public String getMessage(Locale locale, String code) {
        /* code */
    }

    public boolean localeExists(Locale locale) {
        /* code */
    }
}

Sie können sehen, dass es intern "nur eine Karte" ist, aber die öffentliche Oberfläche zeigt dies nicht an. Und es gibt "programmiererfreundliche" Methoden, appendMessage(Locale locale, String code, String message)um neue Einträge einfacher und aussagekräftiger einzufügen. Und Benutzer der Klasse können zum Beispiel nicht translations.clear(), weil Translationsnicht erweitert wird Map.

Optional können Sie einige der benötigten Methoden immer an intern verwendete Zuordnungen delegieren.

user11153
quelle
6

Ich sehe dies als Beispiel für eine sinnvolle Abstraktion. Eine gute Abstraktion hat einige Eigenschaften:

  1. Es verbirgt Implementierungsdetails, die für den Code, der sie verwendet, irrelevant sind.

  2. Es ist nur so komplex, wie es sein muss.

Durch die Erweiterung wird die gesamte Benutzeroberfläche des übergeordneten Elements verfügbar gemacht. In vielen Fällen kann jedoch vieles davon besser ausgeblendet werden. Sie möchten also das tun, was Sebastian Redl vorschlägt, die Komposition der Vererbung vorziehen und eine Instanz des übergeordneten Elements als hinzufügen ein privates Mitglied Ihrer benutzerdefinierten Klasse. Alle Schnittstellenmethoden, die für Ihre Abstraktion sinnvoll sind, können (in Ihrem Fall) problemlos an die innere Sammlung delegiert werden.

Bei einer Beeinträchtigung der Leistung ist es immer eine gute Idee, den Code zuerst auf Lesbarkeit zu optimieren. Wenn eine Beeinträchtigung der Leistung vermutet wird, profilieren Sie den Code, um die beiden Implementierungen zu vergleichen.

Mike Partridge
quelle
4

+1 zu den anderen Antworten hier. Ich möchte auch hinzufügen, dass dies von der DDD-Community (Domain Driven Design) als sehr empfehlenswert erachtet wird. Sie befürworten, dass Ihre Domain und die Interaktionen mit ihr eine semantische Domainbedeutung haben sollten und nicht die zugrunde liegende Datenstruktur. A Map<String, Widget>könnte ein Cache sein, aber es könnte auch etwas anderes sein, was Sie in "Meine nicht so bescheidene Meinung" (IMNSHO) richtig gemacht haben, um zu modellieren, was die Sammlung darstellt, in diesem Fall ein Cache.

Ich füge eine wichtige Änderung hinzu, da der Domänenklassen-Wrapper um die zugrunde liegende Datenstruktur wahrscheinlich auch andere Mitgliedsvariablen oder -funktionen enthalten sollte, die ihn wirklich zu einer Domänenklasse mit Interaktionen und nicht nur zu einer Datenstruktur machen (wenn nur Java Werttypen hätte) , wir werden sie in Java 10 bekommen - versprochen!)

Es wird interessant sein zu sehen, welchen Einfluss die Streams von Java 8 auf all dies haben werden. Ich kann mir vorstellen, dass einige öffentliche Schnittstellen es vorziehen, einen Stream von (gemeinsames Java-Primitiv oder String einfügen) im Gegensatz zu einem Java-Objekt zu behandeln.

Martijn Verburg
quelle