Что лучше - вернуть ImmutableMap или карту?

90

Допустим, я пишу метод, который должен возвращать Map . Например:

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

Поразмыслив над этим некоторое время, я решил, что нет причин изменять эту карту после ее создания. Таким образом, я хотел бы вернуть ImmutableMap .

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

Следует ли мне оставить тип возвращаемого значения как общую карту или указать, что я возвращаю ImmutableMap?

С одной стороны, именно для этого были созданы интерфейсы; чтобы скрыть детали реализации.
С другой стороны, если я оставлю это так, другие разработчики могут упустить тот факт, что этот объект неизменяем. Таким образом, я не достигну главной цели неизменяемых объектов; чтобы сделать код более понятным за счет минимизации количества объектов, которые могут изменяться. Хуже того, через некоторое время кто-то может попытаться изменить этот объект, и это приведет к ошибке выполнения (компилятор не предупредит об этом).

AMT
источник
2
Я подозреваю, что кто-то в конечном итоге отметит этот вопрос как слишком открытый для аргументов. Можете ли вы вместо "WDYT" задать более конкретные вопросы? и "все в порядке ...?" Например, "есть ли какие-либо проблемы или соображения по поводу возврата обычной карты?" и "какие альтернативы существуют?"
ash
Что, если позже вы решите, что он действительно должен возвращать изменяемую карту?
user253751
3
@immibis lol, или список в этом отношении?
djechlin
3
Вы действительно хотите встроить зависимость Guava в свой API? Вы не сможете изменить это без нарушения обратной совместимости.
user2357112 поддерживает Монику,
1
Я считаю, что вопрос слишком схематичный, и многие важные детали отсутствуют. Например, есть ли у вас уже Guava в качестве (видимой) зависимости. Даже если он у вас уже есть, фрагменты кода являются псевдокодом и не передают того, чего вы действительно хотите достичь. Вы бы точно не стали писать return ImmutableMap.of(somethingElse). Вместо этого вы должны сохранить ImmutableMap.of(somethingElse)как поле и вернуть только это поле. Все это здесь влияет на дизайн.
Marco13

Ответы:

70
  • Если вы пишете общедоступный API и эта неизменяемость является важным аспектом вашего дизайна, я определенно сделаю это явным: либо имя метода явно указывает на то, что возвращаемая карта будет неизменной, либо путем возврата конкретного типа карта. На мой взгляд, упоминания об этом в javadoc недостаточно.

    Поскольку вы, очевидно, используете реализацию Guava, я просмотрел документ, и это абстрактный класс, поэтому он дает вам немного гибкости в отношении фактического, конкретного типа.

  • Если вы пишете внутренний инструмент / библиотеку, становится гораздо более приемлемым просто вернуть простой Map. Люди будут знать внутреннее устройство кода, который они вызывают, или, по крайней мере, будут иметь легкий доступ к нему.

Я пришел к выводу, что явное - это хорошо, не оставляйте все на волю случая.

Дичи
источник
1
Один дополнительный интерфейс, один дополнительный абстрактный класс и один класс, расширяющий этот абстрактный класс. Это слишком много хлопот для такой простой вещи, как спрашивает OP, а именно, если метод возвращает Mapтип интерфейса или ImmutableMapтип реализации.
fps
20
Если вы имеете в виду Guava's ImmutableMap, команда Guava советует рассмотреть ImmutableMapинтерфейс с семантическими гарантиями и вам следует возвращать этот тип напрямую.
Луи Вассерман
1
@LouisWasserman мм, не видел, чтобы вопрос давал ссылку на реализацию Guava. Тогда я удалю отрывок, спасибо
Dici
3
Я согласен с Луи, если вы собираетесь вернуть карту, которая является неизменяемым объектом, желательно убедиться, что возвращаемый объект имеет этот тип. Если по какой-то причине вы хотите скрыть тот факт, что это неизменяемый тип, определите свой собственный интерфейс. Оставить это как карту вводит в заблуждение.
WSimpson
1
@WSimpson Я считаю, что это то, о чем я говорю. Луи говорил о добавленном мной фрагменте, но я удалил его.
Dici
38

У вас должен быть ImmutableMapтип возврата. Mapсодержит методы, которые не поддерживаются в реализации ImmutableMap(например put) и отмеченные @deprecatedв ImmutableMap.

Использование устаревших методов приведет к предупреждению компилятора, и большинство IDE будут предупреждать, когда люди пытаются использовать устаревшие методы.

Это расширенное предупреждение предпочтительнее наличия исключений во время выполнения в качестве первого намёка на то, что что-то не так.

Synesso
источник
1
На мой взгляд, это наиболее разумный ответ. Интерфейс карты - это контракт, и ImmutableMap явно нарушает важную его часть. Использование Map в качестве возвращаемого типа в этом случае не имеет смысла.
TonioElGringo
@TonioElGringo, а что Collections.immutableMapтогда?
OrangeDog
@TonioElGringo, обратите внимание, что методы, которые ImmutableMap не реализуют, явно помечены как необязательные в документации интерфейса docs.oracle.com/javase/7/docs/api/java/util/… . Таким образом, я думаю, что все еще имеет смысл возвращать карту (с надлежащими документами javadoc). Несмотря на это, я предпочитаю явно возвращать ImmutableMap по причинам, описанным выше.
AMT
3
@TonioElGringo ничего не нарушает, эти методы не обязательны. Как и в случае List, нет никакой гарантии List::add. Попробуй Arrays.asList("a", "b").add("c");. Нет никаких указаний на то, что он выйдет из строя во время выполнения, но это так. По крайней мере, Гуава предупреждает вас.
njzk2
14

С другой стороны, если я оставлю это так, другие разработчики могут упустить тот факт, что этот объект неизменяем.

Вы должны упомянуть об этом в javadocs. Знаете, разработчики их читают.

Таким образом, я не достигну главной цели неизменяемых объектов; чтобы сделать код более понятным за счет минимизации количества объектов, которые могут изменяться. Хуже того, через некоторое время кто-то может попытаться изменить этот объект, и это приведет к ошибке выполнения (компилятор не предупредит об этом).

Ни один разработчик не публикует свой код непроверенным. И когда он это проверяет, он получает исключение, в котором он не только видит причину, но также файл и строку, в которых он пытался записать на неизменяемую карту.

Обратите внимание, что Mapнеизменным будет только сам объект, а не содержащиеся в нем объекты.

tkausl
источник
10
«Ни один разработчик не публикует свой код непроверенным». Вы хотите сделать ставку на это? Если бы это предложение было верным, в общедоступном программном обеспечении было бы гораздо меньше (как я предполагаю) ошибок. Я бы даже не был уверен, что "большинство разработчиков ..." было бы правдой. И да, это разочаровывает.
Tom
1
Хорошо, Том прав, я не уверен, что вы пытаетесь сказать, но то, что вы на самом деле написали, явно ложно. (Также: подталкивание к гендерной инклюзивности)
djechlin
@Tom, я думаю, вы пропустили сарказм в этом ответе
Alma Alma
10

если я оставлю это так, другие разработчики могут упустить тот факт, что этот объект неизменен

Это правда, но другим разработчикам следует протестировать свой код и убедиться, что он покрыт.

Тем не менее, у вас есть еще 2 варианта решения этой проблемы:

  • Использовать Javadoc

    @return a immutable map
    
  • Выберите описательное имя метода

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

    Для конкретного случая использования вы можете даже лучше назвать методы. Например

    public Map<String, Integer> getUnmodifiableCountByWords()
    

Что еще можно сделать?!

Вы можете вернуть

  • копировать

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

    Этот подход следует использовать, если вы ожидаете, что множество клиентов будут изменять карту, и пока карта содержит только несколько записей.

  • CopyOnWriteMap

    Копирование при записи коллекций обычно используется, когда вам приходится иметь дело с
    параллелизмом. Но эта концепция также поможет вам в вашей ситуации, поскольку CopyOnWriteMap создает копию внутренней структуры данных при изменении операции (например, добавление, удаление).

    В этом случае вам понадобится тонкая оболочка вокруг вашей карты, которая делегирует все вызовы методов базовой карте, кроме изменяющих операций. Если вызывается изменяющая операция, она создает копию базовой карты, и все дальнейшие вызовы будут делегированы этой копии.

    Этот подход следует использовать, если вы ожидаете, что некоторые клиенты изменят карту.

    К сожалению, в java нет такого CopyOnWriteMap. Но вы можете найти третью сторону или реализовать это самостоятельно.

Наконец, вы должны иметь в виду, что элементы на вашей карте все еще могут быть изменяемыми.

Рене Линк
источник
8

Определенно верните ImmutableMap, обоснование:

  • Подпись метода (включая тип возвращаемого значения) должна быть самодокументированной. Комментарии похожи на обслуживание клиентов: если вашим клиентам нужно на них полагаться, значит, ваш основной продукт неисправен.
  • Является ли что-то интерфейсом или классом, имеет значение только при его расширении или реализации. Учитывая экземпляр (объект), в 99% случаев клиентский код не будет знать или не заботиться о том, является ли что-то интерфейсом или классом. Сначала я предположил, что ImmutableMap - это интерфейс. Только после того, как я щелкнул ссылку, я понял, что это класс.
Бенито Чиаро
источник
5

Это зависит от самого класса. Guava ImmutableMapне предназначен для того, чтобы быть неизменным представлением изменяемого класса. Если ваш класс неизменяемый и имеет некоторую структуру, которая в основном является классом ImmutableMap, сделайте возвращаемый тип ImmutableMap. Однако, если ваш класс изменчив, не делайте этого. Если у вас есть это:

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

Гуава будет каждый раз копировать карту. Это медленно. Но если internalMapуже был ImmutableMap, то все нормально.

Если вы не ограничиваете свой класс возвратом ImmutableMap, вы можете вместо этого вернуться Collections.unmodifiableMapтак:

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

Обратите внимание, что это неизменный вид на карту. Если internalMapизменится, то сохранится и кешированная копия Collections.unmodifiableMap(internalMap). Однако я все еще предпочитаю его для геттеров.

Джастин
источник
1
Это хорошие моменты, но для полноты картины мне нравится этот ответ, в котором объясняется, что он не unmodifiableMapможет быть изменен только держателем ссылки - это вид, и держатель вспомогательной карты может изменить вспомогательную карту, а затем вид изменится. Так что это важное соображение.
davidbak
@ Как прокомментировал davidback, будьте очень осторожны при использовании Collections.unmodifiableMap . Этот метод возвращает вид на исходную карту, а не создает отдельный отдельный новый объект карты. Таким образом, любой другой код, ссылающийся на исходную карту, может ее изменить, а затем владелец якобы неизменяемой карты на собственном опыте узнает, что карта действительно может быть изменена из-под него. Я сам был укусил этим фактом. Я научился либо возвращать копию Карты, либо использовать Гуаву ImmutableMap.
Basil Bourque
5

Это не ответ на точный вопрос, но все же стоит подумать, нужно ли вообще возвращать карту. Если карта неизменна, то предоставляемый основной метод основан на get (key):

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

Это делает API намного более жестким. Если карта действительно требуется, это можно оставить на усмотрение клиента API, предоставив поток записей:

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

Затем клиент может создать свою собственную неизменяемую или изменяемую карту по своему усмотрению или добавить записи в свою собственную карту. Если клиенту необходимо знать, существует ли значение, вместо него можно вернуть optional:

public Optional<Integer> fooOf(String key) {
    return Optional.ofNullable(map.get(key));
}
Solubris
источник
-1

Неизменяемая карта - это разновидность карты. Так что оставить возвращаемый тип Map - это нормально.

Чтобы гарантировать, что пользователи не изменят возвращаемый объект, документация метода может описывать характеристики возвращаемого объекта.

торо
источник
-1

Возможно, это вопрос личного мнения, но здесь лучше использовать интерфейс для класса карты. Этот интерфейс не должен явно указывать, что он неизменяемый, но сообщение будет таким же, если вы не предоставите ни один из методов установки родительского класса в интерфейсе.

Взгляните на следующую статью:

Энди Гибсон

WSimpson
источник
1
Ненужные обертки считаются вредными.
djechlin
Вы правы, я бы и здесь не стал использовать обертку, так как интерфейса достаточно.
WSimpson,
У меня есть ощущение, что если бы это было правильно, ImmutableMap уже реализовал бы ImmutableMapInterface, но я не понимаю этого технически.
djechlin
Дело не в том, что планировали разработчики Guava, а в том, что этот разработчик хотел бы инкапсулировать архитектуру и не раскрывать использование ImmutableMap. Один из способов сделать это - использовать интерфейс. Использование обертки, как вы указали, не обязательно и поэтому не рекомендуется. Возвращать карту нежелательно, так как она вводит в заблуждение. Таким образом, кажется, что если целью является инкапсуляция, то интерфейс - лучший вариант.
WSimpson
Недостатком возврата чего-то, что не расширяет (или не реализует) Mapинтерфейс, является то, что вы не можете передать это какой-либо функции API, ожидающей a, Mapбез ее преобразования. Сюда входят всевозможные копи-конструкторы других типов карт. В дополнение к этому, если изменчивость только «скрыта» интерфейсом, ваши пользователи всегда могут обойти это, преобразовав и взломав ваш код таким образом.
Hulk