Apakah terlalu berlebihan untuk membungkus koleksi dalam kelas sederhana hanya demi keterbacaan yang lebih baik?

15

Saya memiliki peta berikut:

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

Ini HashMapmemetakan doublenilai (yang merupakan titik waktu) ke SoundEvent'sel' yang sesuai : setiap 'sel' dapat berisi sejumlah SoundEvents. Itu sebabnya ini diimplementasikan sebagai List<SoundEvent>, karena memang seperti itu.

Demi keterbacaan kode yang lebih baik, saya berpikir untuk mengimplementasikan kelas internal statis yang sangat sederhana seperti:

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
}

Dan daripada deklarasi peta (dan banyak kode lainnya) akan terlihat lebih baik, misalnya:

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

Apakah ini berlebihan? Apakah Anda melakukan ini dalam proyek Anda?

Aviv Cohn
sumber
orang mungkin berpendapat bahwa secara konseptual, ini telah dibahas dalam Bagaimana Anda tahu jika Anda telah menulis kode yang mudah dibaca dan mudah dirawat? Jika teman sebaya Anda terus mengeluh tentang cara Anda melakukan sesuatu, baik itu satu atau lain cara, Anda lebih baik berubah untuk membuat mereka merasa lebih baik
Agak
1
Apa yang membuat daftar peristiwa suara menjadi "sel" dan bukan daftar? Apakah pilihan kata-kata ini berarti bahwa sel memiliki atau pada akhirnya akan memiliki perilaku yang berbeda dari daftar?
x-code
@DocBrown Mengapa? Kelas private statickarena itu hanya akan digunakan oleh kelas luar, tetapi tidak terkait dengan contoh spesifik dari kelas luar. Bukankah itu penggunaan yang tepat private static?
Aviv Cohn
2
@ Doc Brown, Aviv Cohn: Tidak ada tag yang menentukan bahasa apa pun, jadi apa pun bisa benar dan salah pada saat yang sama!
Emilio Garavaglia
@EmilioGaravaglia: Java (saya pikir sudah cukup jelas karena dilihat dari sintaksnya bisa berupa Java atau C #, dan konvensi yang digunakan mempersempitnya ke Jawa;)).
Aviv Cohn

Jawaban:

12

Sama sekali tidak berlebihan. Mulailah dengan operasi yang Anda butuhkan, daripada memulai dengan "Saya bisa menggunakan HashMap". Terkadang, HashMap adalah yang Anda butuhkan.
Dalam kasus Anda, saya kira tidak. Yang mungkin ingin Anda lakukan adalah sesuatu seperti ini:

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

Anda pasti tidak ingin memiliki banyak kode yang mengatakan ini:

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

Atau mungkin Anda bisa menggunakan salah satu implementasi Multimap Guava .

kevin cline
sumber
Jadi Anda menganjurkan menggunakan kelas, yang pada dasarnya merupakan mekanisme penyembunyian informasi, sebagai ... mekanisme penyembunyian informasi? Menyeramkan.
Robert Harvey
1
Sebenarnya ya, saya memiliki TimeLinekelas tepat untuk hal semacam itu :) Ini pembungkus tipis sekitar HashMap<Double, SoundEventCell>(akhirnya aku pergi dengan SoundEventCellbukan List<SoundEvent>ide). Jadi saya hanya bisa melakukan timeline.addEvent(4.5, new SoundEvent(..))dan memiliki hal-hal tingkat rendah dienkapsulasi :)
Aviv Cohn
14

Meskipun dapat membantu keterbacaan di beberapa daerah, hal ini juga dapat memperumit masalah. Saya pribadi menjauh dari membungkus atau memperluas koleksi demi kelancaran, karena pembungkus baru, pada bacaan awal, menyiratkan kepada saya bahwa mungkin ada perilaku yang perlu saya waspadai. Anggap itu sebagai naungan Prinsip Terkejut.

Tetap dengan implementasi antarmuka berarti saya hanya perlu khawatir tentang antarmuka. Implementasi konkretnya, tentu saja, dapat menampung perilaku tambahan, tetapi saya tidak perlu khawatir tentang hal itu. Jadi, ketika saya mencoba mencari jalan melalui kode seseorang, saya lebih suka antarmuka yang mudah dibaca.

Jika, di sisi lain, Anda menemukan kasus penggunaan yang tidak manfaat dari perilaku menambahkan, maka Anda memiliki argumen untuk meningkatkan kode dengan menciptakan kelas matang penuh.

FeedbackLoop
sumber
11
Pembungkus juga dapat digunakan untuk menghapus (atau menyembunyikan) perilaku yang tidak dibutuhkan.
Roman Reiner
4
@RomanReiner - Saya akan mengingatkan hal-hal seperti itu. Perilaku yang tidak dibutuhkan hari ini seringkali programmer mengutuk nama Anda besok. Semua orang tahu apa yang Listbisa dilakukan, dan melakukan semua itu untuk alasan yang baik.
Telastyn
Saya menghargai keinginan untuk mempertahankan fungsionalitas, meskipun saya pikir solusinya adalah keseimbangan yang cermat antara fungsionalitas dan abstraksi. SoundEventCelldapat menerapkan Iterableuntuk SoundEvents, yang akan menawarkan iterator soundEventsanggota, sehingga Anda akan dapat membaca (tetapi tidak menulis) sebagai daftar apa pun. Saya ragu untuk menutupi kerumitan hampir sebanyak saya ragu untuk menggunakan Listketika saya mungkin membutuhkan sesuatu yang lebih dinamis di masa depan.
Neil
2

Membungkusnya membatasi fungsionalitas Anda hanya pada metode yang Anda putuskan untuk ditulis, pada dasarnya meningkatkan kode Anda tanpa manfaat. Paling tidak, saya akan mencoba yang berikut:

private static class SoundEventCell : List<SoundEvent>
{
}

Anda masih dapat menulis kode dari contoh Anda.

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

Yang mengatakan, saya hanya pernah melakukan ini ketika ada beberapa fungsi yang dibutuhkan daftar itu sendiri. Tapi saya pikir metode Anda akan berlebihan dalam hal ini. Kecuali Anda memiliki alasan untuk membatasi akses ke sebagian besar metode Daftar.

Lawtonfogle
sumber
-1

Solusi lain mungkin mendefinisikan kelas pembungkus Anda dengan metode tunggal yang memaparkan daftar:

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

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

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

Ini memberi Anda kelas yang dinamai dengan kode minimal, tetapi masih memberi Anda enkapsulasi, memungkinkan Anda untuk membuat misal membuat kelas tidak berubah (dengan melakukan salinan defensif di konstruktor dan menggunakan Collections.unmodifiableListdi accessor).

(Namun, jika daftar ini memang hanya digunakan di kelas ini, saya pikir Anda akan lebih baik untuk mengganti Anda Map<Double, List<SoundEvent>>dengan Multimap<Double, SoundEvent>( docs ), karena itu sering menyimpan banyak logika dan kesalahan pemeriksaan-nol.)

MikeFHay
sumber