Ist es übertrieben, eine Sammlung nur aus Gründen der besseren Lesbarkeit in eine einfache Klasse zu packen?

Ich habe folgende Karte:

Map<Double, List<SoundEvent>> soundEventCells = new HashMap<Double, List<SoundEvent>>();

Dies HashMapordnet doubleWerte (die Zeitpunkte sind) der entsprechenden SoundEvent'Zelle' zu: Jede 'Zelle' kann eine Anzahl von SoundEvents enthalten. Deshalb ist es als implementiert List<SoundEvent>, weil es genau das ist, was es ist.

Um den Code besser lesbar zu machen, habe ich mir überlegt, eine sehr einfache statische innere Klasse wie die folgende zu implementieren:

private static class SoundEventCell {
    private List<SoundEvent> soundEvents = new ArrayList<SoundEvent>();
    public void addEvent(SoundEvent event){
        soundEvents.add(event);
    }
    public int getSize(){
        return soundEvents.size();
    }
    public SoundEvent getEvent(int index){
        return soundEvents.get(index);
    }
    // .. remove() method unneeded
}

Und dann würde die Kartendeklaration (und viele andere Codes) besser aussehen, zum Beispiel:

Map<Double, SoundEventCell> soundEventCells = new HashMap<Double, SoundEventCell>();

Ist das übertrieben? Würden Sie dies in Ihren Projekten tun?

Es ist überhaupt nicht übertrieben. Beginnen Sie mit den Operationen, die Sie benötigen, anstatt mit "Ich kann eine HashMap verwenden" zu beginnen. Manchmal ist eine HashMap genau das, was Sie brauchen.
Ich vermute, dass es in Ihrem Fall nicht so ist. Was Sie wahrscheinlich tun möchten, ist etwa so:

public class EventsByTime {
    public EventsByTime addEvent(double time, SoundEvent e);
    public List<SoundEvent> getEvents(double time);
    // ... more methods specific to your use ...
}

Sie möchten definitiv keinen Code haben, der dies sagt:

List<SoundEvent> events = eventMap.get(time);
if (events == null) {
   events = new ArrayList<SoundEvent>();
   eventMap.put(time, events);
}

Oder Sie könnten einfach eine der Guava Multimap- Implementierungen verwenden.

In einigen Bereichen kann dies die Lesbarkeit verbessern, aber auch die Sache komplizieren. Ich persönlich lehne es aus Gründen der Übersichtlichkeit ab, Sammlungen zu verpacken oder zu erweitern, da der neue Umschlag beim ersten Lesen für mich impliziert, dass es möglicherweise ein Verhalten gibt, dessen ich mich bewusst sein muss. Betrachten Sie es als eine Nuance des Prinzips der geringsten Überraschung.

Wenn ich mich an die Schnittstellenimplementierung halte, muss ich mich nur um die Schnittstelle kümmern. Die konkrete Implementierung kann natürlich zusätzliches Verhalten beinhalten, aber ich sollte mir darüber keine Sorgen machen müssen. Wenn ich also versuche, mich im Code von jemandem zurechtzufinden, bevorzuge ich die einfachen Schnittstellen für die Lesbarkeit.

Wenn Sie auf der anderen Seite einen Anwendungsfall finden, der von zusätzlichem Verhalten profitiert, haben Sie ein Argument für die Verbesserung des Codes, indem Sie eine vollwertige Klasse erstellen.

Durch das Umschließen wird Ihre Funktionalität auf die Methoden beschränkt, für die Sie sich zum Schreiben entschieden haben, und der Code wird im Grunde genommen ohne Nutzen erhöht. Zumindest würde ich Folgendes versuchen:

private static class SoundEventCell : List<SoundEvent>
{
}

Sie können weiterhin den Code aus Ihrem Beispiel schreiben.

Map<Double, SoundEventCell> soundEventCells = new HashMap<Double, SoundEventCell>();

Das habe ich allerdings nur getan, wenn es einige Funktionen gibt, die die Liste selbst benötigt. Aber ich denke, Ihre Methode wäre übertrieben. Es sei denn, Sie hatten einen Grund, den Zugriff auf die meisten Methoden von List einzuschränken.

Eine andere Lösung könnte darin bestehen, Ihre Wrapper-Klasse mit einer einzigen Methode zu definieren, die die Liste verfügbar macht:

private static class SoundEventCell
{
    private List<SoundEvent> events;

    public SoundEventCell(List<SoundEvent> events)
    {
        this.events = events;
    }

    public List<SoundEvent> getEvents()
    {
        return events;
    }
}

Dies gibt Ihnen Ihre gut benannte Klasse mit minimalem Code, bietet Ihnen aber dennoch eine Kapselung, mit der Sie z. B. die Klasse unveränderlich machen können (indem Sie eine defensive Kopie im Konstruktor erstellen und Collections.unmodifiableListim Accessor verwenden).

(Wenn diese Listen jedoch nur in dieser Klasse verwendet werden, sollten Sie sie meines Erachtens besser durch Map<Double, List<SoundEvent>>ein Multimap<Double, SoundEvent>( docs ) ersetzen , da dies häufig viel Logik und Fehler bei der Nullprüfung erspart.)