Apakah lebih baik mengembalikan ImmutableMap atau Map?

90

Katakanlah saya sedang menulis metode yang harus mengembalikan Peta . Contohnya:

public Map<String, Integer> foo() {
  return new HashMap<String, Integer>();
}

Setelah memikirkannya sebentar, saya memutuskan bahwa tidak ada alasan untuk memodifikasi Peta ini setelah dibuat. Jadi, saya ingin mengembalikan sebuah ImmutableMap .

public Map<String, Integer> foo() {
  return ImmutableMap.of();
}

Haruskah saya membiarkan tipe pengembalian sebagai Peta generik, atau haruskah saya menentukan bahwa saya mengembalikan ImmutableMap?

Dari satu sisi, inilah tepatnya mengapa antarmuka dibuat untuk; untuk menyembunyikan detail implementasi.
Di sisi lain, jika saya membiarkannya seperti ini, pengembang lain mungkin melewatkan fakta bahwa objek ini tidak dapat diubah. Jadi, saya tidak akan mencapai tujuan utama dari objek yang tidak bisa diubah; untuk membuat kode lebih jelas dengan meminimalkan jumlah objek yang dapat berubah. Lebih buruk lagi, setelah beberapa saat, seseorang mungkin mencoba untuk mengubah objek ini, dan ini akan menghasilkan kesalahan waktu proses (Kompilator tidak akan memperingatkannya).

AMT
sumber
2
Saya curiga seseorang pada akhirnya akan menandai pertanyaan ini sebagai terlalu terbuka untuk argumen. Bisakah Anda memasukkan pertanyaan yang lebih spesifik daripada "WDYT?" dan "apakah OK ...?" Misalnya, "apakah ada masalah atau pertimbangan dalam mengembalikan peta biasa?" dan "alternatif apa yang ada?"
abu
Bagaimana jika nanti Anda memutuskan itu benar-benar harus mengembalikan peta yang bisa berubah?
pengguna253751
3
@ imibis lol, atau Daftar dalam hal ini?
djechlin
3
Apakah Anda benar-benar ingin memasukkan ketergantungan Jambu ke dalam API Anda? Anda tidak akan bisa mengubahnya tanpa merusak kompatibilitas ke belakang.
user2357112 mendukung Monica
1
Saya pikir pertanyaannya terlalu samar, dan banyak detail penting yang hilang. Misalnya, apakah Anda sudah memiliki Guava sebagai dependensi (terlihat). Meskipun Anda sudah memilikinya, potongan kode tersebut adalah kode pesudocode, dan tidak menyampaikan apa yang sebenarnya ingin Anda capai. Anda pasti tidak akan menulis return ImmutableMap.of(somethingElse). Sebaliknya, Anda akan menyimpan ImmutableMap.of(somethingElse)sebagai bidang, dan hanya mengembalikan bidang ini. Semua ini memengaruhi desain di sini.
Marco13

Jawaban:

70
  • Jika Anda menulis API yang dihadapi publik dan keabadian itu merupakan aspek penting dari desain Anda, saya pasti akan membuatnya eksplisit baik dengan memiliki nama metode yang secara jelas menunjukkan bahwa peta yang dikembalikan tidak dapat diubah atau dengan mengembalikan jenis konkret dari peta. Menyebutnya di javadoc saja tidak cukup menurut saya.

    Karena Anda tampaknya menggunakan implementasi Guava, saya melihat doc dan ini adalah kelas abstrak sehingga memberi Anda sedikit fleksibilitas pada tipe konkret yang sebenarnya.

  • Jika Anda menulis alat / pustaka internal, akan jauh lebih dapat diterima untuk hanya mengembalikan polos Map. Orang-orang akan tahu tentang bagian dalam kode yang mereka panggil atau setidaknya akan memiliki akses yang mudah ke sana.

Kesimpulan saya adalah eksplisit itu baik, jangan biarkan hal-hal kebetulan.

Dici
sumber
1
Satu antarmuka ekstra, satu kelas abstrak ekstra, dan satu kelas yang memperluas kelas abstrak ini. Ini terlalu merepotkan untuk hal sederhana seperti yang diminta OP, yaitu jika metode harus mengembalikan Maptipe antarmuka atau ImmutableMaptipe implementasi.
fps
20
Jika Anda merujuk ke Guava's ImmutableMap, saran tim Guava adalah mempertimbangkan ImmutableMapantarmuka dengan jaminan semantik, dan Anda harus mengembalikan tipe tersebut secara langsung.
Louis Wasserman
1
@LouisWasserman mm, tidak melihat pertanyaan yang memberikan link ke implementasi Guava. Saya akan menghapus cuplikannya, terima kasih
Dici
3
Saya setuju dengan Louis, jika Anda bermaksud untuk mengembalikan peta yang merupakan objek yang tidak dapat diubah, pastikan bahwa objek yang dikembalikan adalah jenis yang lebih disukai. Jika karena alasan tertentu Anda ingin mengaburkan fakta bahwa ini adalah tipe yang tidak dapat diubah, tentukan antarmuka Anda sendiri. Membiarkannya sebagai Peta menyesatkan.
WSimpson
1
@WSimpson Saya percaya itulah jawaban saya. Louis berbicara tentang beberapa cuplikan yang telah saya tambahkan, tetapi saya menghapusnya.
Dici
38

Anda harus memiliki ImmutableMaptipe pengembalian Anda. Mapberisi metode yang tidak didukung oleh pelaksanaan ImmutableMap(misalnya put) dan ditandai @deprecateddi ImmutableMap.

Menggunakan metode yang tidak berlaku lagi akan menghasilkan peringatan compiler & sebagian besar IDE akan memberi peringatan ketika orang mencoba menggunakan metode yang tidak digunakan lagi.

Peringatan lanjutan ini lebih disukai daripada memiliki pengecualian waktu proses sebagai petunjuk pertama Anda bahwa ada sesuatu yang salah.

Synesso
sumber
1
Menurut saya, ini adalah jawaban yang paling masuk akal. Antarmuka Map adalah sebuah kontrak, dan ImmutableMap jelas melanggar bagian penting darinya. Menggunakan Map sebagai tipe kembali dalam kasus ini tidak masuk akal.
TonioElGringo
@TonioElGringo bagaimana dengan Collections.immutableMapitu?
OrangeDog
@TonioElGringo harap perhatikan bahwa metode yang tidak diterapkan oleh ImmutableMap secara eksplisit ditandai sebagai opsional dalam dokumentasi antarmuka docs.oracle.com/javase/7/docs/api/java/util/… . Jadi, saya pikir masih masuk akal untuk mengembalikan Peta (dengan javadocs yang tepat). Meskipun, saya memilih untuk mengembalikan ImmutableMap secara eksplisit, karena alasan yang dibahas di atas.
AMT
3
@TonioElGringo itu tidak melanggar apa pun, metode itu opsional. Seperti di List, tidak ada jaminan yang List::addvalid. Coba Arrays.asList("a", "b").add("c");. Tidak ada indikasi bahwa itu akan gagal saat runtime, namun gagal. Setidaknya Jambu biji memberi Anda perhatian.
njzk2
14

Di sisi lain, jika saya membiarkannya seperti ini, pengembang lain mungkin melewatkan fakta bahwa objek ini tidak dapat diubah.

Anda harus menyebutkannya di javadocs. Pengembang memang membacanya, Anda tahu.

Jadi, saya tidak akan mencapai tujuan utama dari objek yang tidak bisa diubah; untuk membuat kode lebih jelas dengan meminimalkan jumlah objek yang dapat berubah. Lebih buruk lagi, setelah beberapa saat, seseorang mungkin mencoba untuk mengubah objek ini, dan ini akan menghasilkan kesalahan waktu proses (Kompilator tidak akan memperingatkannya).

Tidak ada pengembang yang menerbitkan kodenya belum teruji. Dan ketika dia mengujinya, dia mendapatkan Exception yang tidak hanya melihat alasannya tetapi juga file dan baris tempat dia mencoba menulis ke peta yang tidak dapat diubah.

Namun perlu dicatat, hanya objek Mapitu sendiri yang tidak dapat diubah, bukan objek yang dikandungnya.

tkausl
sumber
10
"Tidak ada pengembang yang menerbitkan kodenya tanpa diuji." Apakah Anda ingin bertaruh untuk ini? Akan jauh lebih sedikit bug (anggapan saya) dalam perangkat lunak publik, jika kalimat ini benar. Saya bahkan tidak yakin "sebagian besar pengembang ..." akan benar. Dan ya, itu mengecewakan.
Tom
1
Oke, Tom benar, saya tidak yakin apa yang Anda coba katakan, tetapi apa yang sebenarnya Anda tulis jelas salah. (Juga: dorongan untuk inklusivitas gender)
djechlin
@Tom Saya kira Anda melewatkan sarkasme dalam jawaban ini
Kapten Fogetti
10

jika saya membiarkannya seperti ini, pengembang lain mungkin melewatkan fakta bahwa objek ini tidak dapat diubah

Itu benar, tetapi pengembang lain harus menguji kode mereka dan memastikan bahwa itu tercakup.

Namun demikian, Anda memiliki 2 opsi lagi untuk menyelesaikan ini:

  • Gunakan Javadoc

    @return a immutable map
    
  • Pilih nama metode deskriptif

    public Map<String, Integer> getImmutableMap()
    public Map<String, Integer> getUnmodifiableEntries()
    

    Untuk kasus penggunaan konkret, Anda bahkan dapat memberi nama metode dengan lebih baik. Misalnya

    public Map<String, Integer> getUnmodifiableCountByWords()
    

Apa lagi yang bisa kamu lakukan ?!

Anda dapat mengembalikan file

  • salinan

    private Map<String, Integer> myMap;
    
    public Map<String, Integer> foo() {
      return new HashMap<String, Integer>(myMap);
    }
    

    Pendekatan ini harus digunakan jika Anda berharap banyak klien akan memodifikasi peta dan selama peta hanya berisi sedikit entri.

  • CopyOnWriteMap

    salinan pada koleksi tulis biasanya digunakan ketika Anda harus berurusan dengan
    konkurensi. Tetapi konsep tersebut juga akan membantu Anda dalam situasi Anda, karena CopyOnWriteMap membuat salinan dari struktur data internal pada operasi mutatif (misalnya menambah, menghapus).

    Dalam hal ini Anda memerlukan pembungkus tipis di sekitar peta Anda yang mendelegasikan semua pemanggilan metode ke peta yang mendasarinya, kecuali operasi mutatif. Jika operasi mutatif dipanggil, itu membuat salinan dari peta yang mendasarinya dan semua pemanggilan lebih lanjut akan didelegasikan ke salinan ini.

    Pendekatan ini harus digunakan jika Anda mengharapkan bahwa beberapa klien akan memodifikasi peta.

    Sayangnya java tidak memiliki file CopyOnWriteMap. Tetapi Anda mungkin menemukan pihak ketiga atau menerapkannya sendiri.

Terakhir, Anda harus ingat bahwa elemen di peta Anda mungkin masih bisa berubah.

René Link
sumber
8

Pasti mengembalikan ImmutableMap, pembenarannya adalah:

  • Tanda tangan metode (termasuk tipe kembalian) harus mendokumentasikan sendiri. Komentar itu seperti layanan pelanggan: jika klien Anda perlu mengandalkan mereka, maka produk utama Anda rusak.
  • Apakah sesuatu itu antarmuka atau kelas hanya relevan saat memperluas atau mengimplementasikannya. Diberikan sebuah contoh (objek), 99% dari kode klien waktu tidak akan tahu atau peduli apakah sesuatu itu antarmuka atau kelas. Saya berasumsi pada awalnya bahwa ImmutableMap adalah sebuah antarmuka. Hanya setelah saya mengklik link tersebut, saya menyadari bahwa ini adalah sebuah kelas.
Benito Ciaro
sumber
5

Itu tergantung kelasnya sendiri. Guava's ImmutableMaptidak dimaksudkan untuk menjadi tampilan yang tidak dapat diubah ke dalam kelas yang bisa berubah. Jika kelas Anda tidak dapat diubah dan memiliki beberapa struktur yang pada dasarnya adalah an ImmutableMap, buatlah tipe yang dikembalikan ImmutableMap. Namun, jika kelas Anda bisa berubah, jangan. Jika Anda memiliki ini:

public ImmutableMap<String, Integer> foo() {
    return ImmutableMap.copyOf(internalMap);
}

Guava akan menyalin peta setiap saat. Itu lambat. Tapi kalau internalMapsudah jadi ImmutableMap, maka tidak apa-apa.

Jika Anda tidak membatasi kelas untuk kembali ImmutableMap, Anda dapat kembali Collections.unmodifiableMapseperti ini:

public Map<String, Integer> foo() {
    return Collections.unmodifiableMap(internalMap);
}

Perhatikan bahwa ini adalah tampilan yang tidak dapat diubah ke dalam peta. Jika internalMapberubah, begitu juga salinan yang disimpan dalam cache Collections.unmodifiableMap(internalMap). Saya masih lebih suka untuk getter.

Justin
sumber
1
Ini adalah poin yang bagus, tetapi untuk kelengkapan, saya suka jawaban ini yang menjelaskan bahwa unmodifiableMaphanya tidak dapat dimodifikasi oleh pemegang referensi - ini adalah tampilan dan pemegang peta pendukung dapat memodifikasi peta pendukung dan kemudian tampilan berubah. Jadi itu pertimbangan penting.
davidbak
@ Seperti yang dikomentari davidback, berhati-hatilah saat menggunakannya Collections.unmodifiableMap. Metode itu mengembalikan tampilan ke Peta asli daripada membuat objek peta baru yang terpisah. Jadi kode lain yang mereferensikan Peta asli dapat memodifikasinya, dan kemudian pemegang peta yang seharusnya tidak dapat dimodifikasi itu belajar dengan cara yang sulit bahwa peta memang dapat dimodifikasi dari bawahnya. Saya sendiri telah digigit oleh fakta itu. Saya belajar untuk mengembalikan salinan Peta atau menggunakan Jambu ImmutableMap.
Basil Bourque
5

Ini tidak menjawab pertanyaan persisnya, tetapi masih perlu dipertimbangkan apakah sebuah peta harus dikembalikan atau tidak. Jika peta tidak dapat diubah, maka metode utama yang akan disediakan didasarkan pada get (key):

public Integer fooOf(String key) {
    return map.get(key);
}

Ini membuat API lebih ketat. Jika peta benar-benar diperlukan, ini dapat diserahkan kepada klien API dengan menyediakan aliran entri:

public Stream<Map.Entry<String, Integer>> foos() {
    map.entrySet().stream()
}

Kemudian klien dapat membuat petanya sendiri yang tidak dapat diubah atau berubah sesuai kebutuhan, atau menambahkan entri ke petanya sendiri. Jika klien perlu mengetahui apakah nilainya ada, opsional dapat dikembalikan:

public Optional<Integer> fooOf(String key) {
    return Optional.ofNullable(map.get(key));
}
Solubris
sumber
-1

Peta Abadi adalah jenis Peta. Jadi meninggalkan jenis Map kembali tidak apa-apa.

Untuk memastikan bahwa pengguna tidak mengubah objek yang dikembalikan, metode dokumentasi dapat mendeskripsikan karakteristik objek yang dikembalikan.

toro
sumber
-1

Ini bisa dibilang masalah opini, tapi ide yang lebih baik di sini adalah menggunakan antarmuka untuk kelas peta. Antarmuka ini tidak perlu secara eksplisit mengatakan bahwa itu tidak dapat diubah, tetapi pesannya akan sama jika Anda tidak mengekspos metode penyetel apa pun dari kelas induk di antarmuka.

Simak artikel berikut ini:

andy gibson

WSimpson
sumber
1
Pembungkus yang tidak perlu dianggap berbahaya.
djechlin
Anda benar, saya juga tidak akan menggunakan pembungkus di sini, karena antarmuka sudah cukup.
WSimpson
Saya punya perasaan jika ini adalah hal yang benar untuk dilakukan maka ImmutableMap akan mengimplementasikan ImmutableMapInterface sudah, tapi saya tidak mengerti ini secara teknis.
djechlin
Intinya bukanlah apa yang diinginkan oleh pengembang Guava, ini adalah fakta bahwa pengembang ini ingin merangkum arsitektur dan tidak mengekspos penggunaan ImmutableMap. Salah satu cara untuk melakukannya adalah dengan menggunakan antarmuka. Menggunakan pembungkus seperti yang Anda tunjukkan tidak perlu dan karena itu tidak disarankan. Returning Map tidak diinginkan karena menyesatkan. Jadi tampaknya jika tujuannya adalah enkapsulasi, maka Antarmuka adalah pilihan terbaik.
WSimpson
Kerugian dari mengembalikan sesuatu yang tidak memperluas (atau mengimplementasikan) Mapantarmuka adalah Anda tidak dapat meneruskannya ke fungsi API apa pun yang mengharapkan a Maptanpa mentransmisikannya. Ini mencakup semua jenis konstruktor salinan dari jenis peta lainnya. Selain itu, jika mutabilitas hanya "disembunyikan" oleh antarmuka, pengguna Anda selalu dapat mengatasinya dengan mentransmisikan dan memecahkan kode Anda seperti itu.
Hulk