Несколько записей с одной и той же ключевой ошибкой неизменяемой карты

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

public final class ClientKey {
  private final long userId;
  private final int clientId;
  private final String processName;
  private final Map<String, String> parameterMap;

  private ClientKey(Builder builder) {
    this.userId = builder.userId;
    this.clientId = builder.clientId;
    this.processName = builder.processName;
    // initializing the required fields
    // and below line throws exception once I try to clone the `ClientKey` object
    builder.parameterMap.put("is_clientid", (clientId == 0) ? "false" : "true");
    this.parameterMap = builder.parameterMap.build();
  }

  public static class Builder {
    private final long userId;
    private final int clientId;
    private String processName;
    private ImmutableMap.Builder<String, String> parameterMap = ImmutableMap.builder();

    // this is for cloning
    public Builder(ClientKey key) {
      this.userId = key.userId;
      this.clientId = key.clientId;
      this.processName = key.processName;
      this.parameterMap =
          ImmutableMap.<String, String>builder().putAll(key.parameterMap);
    }

    public Builder(long userId, int clientId) {
      this.userId = userId;
      this.clientId = clientId;
    }

    public Builder parameterMap(Map<String, String> parameterMap) {
      this.parameterMap.putAll(parameterMap);
      return this;
    }

    public Builder processName(String processName) {
      this.processName = processName;
      return this;
    }

    public ClientKey build() {
      return new ClientKey(this);
    }
  }

  // getters
}

Ниже показано, как я делаю ClientKey, и он отлично работает.

Map<String, String> testMap = new HashMap<String, String>();
testMap.put("hello", "world");
ClientKey keys = new ClientKey.Builder(12345L, 200).parameterMap(testMap).build();

Теперь, когда я пытаюсь клонировать объект keys, как показано ниже, возникает исключение.

ClientKey clonedKey = new ClientKey.Builder(keys).processName("hello").build();

Выдает исключение с сообщением об ошибке: java.lang.IllegalArgumentException: Multiple entries with same key: is_clientid=true and is_clientid=true

builder.parameterMap.put("is_clientid", (clientId == 0) ? "false" : "true");
// from below line exception is coming
this.parameterMap = builder.parameterMap.build();

Как я могу решить эту проблему? Я хочу сделать свою карту неизменной, но я также хочу инициализировать обязательные поля, и я могу сделать это только в конструкторе класса ClientKey. И это вызывает исключение при клонировании объекта ClientKey.


person Community    schedule 17.12.2016    source источник
comment
не могли бы вы уточнить, где выдается сообщение об ошибке? Из приведенного выше кода неясно, что ClientKey clonedKey = new ClientKey.Builder(keys).processName("hello").build(); вызовет исключение, поскольку у вас даже нет ключа is_clientid. Где происходит следующая строка в вашем коде?   -  person jamesw1234    schedule 17.12.2016
comment
Вы уверены, что это действительно экземпляр шаблона Builder? -- Я никогда не видел, чтобы кто-то передал конструктор частному конструктору только для того, чтобы установить поля нового объекта.   -  person errantlinguist    schedule 17.12.2016
comment
Исключение @jamesw1234 возникает в этой строке this.parameterMap = builder.parameterMap.build();, но только если я пытаюсь клонировать объект keys в новый clonedKey   -  person    schedule 17.12.2016
comment
Так как во время клонирования мой конструктор ClientKey будет вызываться снова, а затем он попытается снова вставить тот же is_clientid в карту, и поэтому он выдает исключение.   -  person    schedule 17.12.2016
comment
где создан экземпляр builder?   -  person jamesw1234    schedule 17.12.2016
comment
проверьте этот метод build() в моем шаблоне строителя. Я предполагаю, что эта ошибка связана с тем, что вы не можете дважды вставить один и тот же ключ в Guava ImmutableMap?   -  person    schedule 17.12.2016
comment
ваш метод build() на самом деле не создает экземпляр builder. В вашем коде builder.parameterMap.build(); где builder создается первый экземпляр   -  person jamesw1234    schedule 17.12.2016
comment
@ jamesw1234 Я не понимаю, что ты имеешь в виду. Если вы скопируете и вставите весь код и попробуете его, вы сможете очень легко воспроизвести это.   -  person    schedule 17.12.2016


Ответы (2)


Когда вы создаете ClientKey, ключ "is_clientid" помещается в карту. Поэтому, если вы вызовете свой конструктор ClientKey.Builder(ClientKey), вызов putAll скопирует его в ваш новый экземпляр ImmutableMap.Builder. Когда вы затем создадите свой клонированный ClientKey, конструктор ClientKey снова попытается добавить тот же ключ к карте, что вызовет исключение.

ImmutableMap.Builder можно было написать по-другому, но это не так. Если вы хотите использовать его, вам придется с этим жить.

Одно из решений — не копировать запись с ключом "is_clientid" в новый ImmutableMap.Builder в конструкторе вашего Builder. Вместо this.parameterMap = ImmutableMap.<String, String>builder().putAll(key.parameterMap); пишешь:

this.parameterMap = new ImmutableMap.Builder<>();
for (Map.Entry<String,String> entry : key.parameterMap.entrySet()) {
    if (!"is_clientid".equals(entry.getKey()) {
        this.parameterMap.put(entry.getKey(), entry.getValue());
    }
}

Другое решение состоит в том, чтобы не использовать ImmutableMap.Builder Guava, а обычный Java HashMap (он не генерирует исключение, когда вы пытаетесь поместить в него дублирующийся ключ, старая запись просто перезаписывается). Затем в своем конструкторе ClientKey вы пишете:

this.parameterMap = Collections.unmodifiableMap(builder.parameterMap);

Вы также можете написать:

this.parameterMap = ImmutableMap.copyOf(builder.parameterMap);

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

Заключительное замечание: если все, что вы хотите сделать, это скопировать ClientKey, вам не нужен компоновщик; идиоматическая Java будет использовать конструктор копирования или метод clone() (хотя последний не рекомендуется некоторыми).

person Hoopje    schedule 17.12.2016
comment
Где я должен использовать первое решение, которое вы упомянули? Это также в конструкторе ClientKey или конструкторе Builder? - person ; 17.12.2016
comment
@ user5447339 Я отредактировал ответ. Фрагмент появится в конструкторе вашего класса Builder. - person Hoopje; 17.12.2016
comment
не использовать ImmutableMap Гуавы - проблема в билдере, а не в карте. Если результат должен быть неизменным и необходимо обрабатывать дубликаты ключей, то его можно построить с помощью ImmutableMap.copyOf(aHashMapUsedAsBuilder). - person maaartinus; 18.12.2016
comment
@maartinus Верно. Я отредактировал ответ. Тем не менее, ImmutableMap.copyOf создает копию всей карты, тогда как Collections.unmodifiableMap создает только вид исходной карты. - person Hoopje; 18.12.2016

Вы получаете исключение, потому что пытаетесь установить значение для ключа is_clientid в том же ImmutableMap.Builder, который используется вашим единственным классом ClientKey.Builder:

builder.parameterMap.put("is_clientid", (clientId == 0) ? "false" : "true");

Как показано в документация:

Связывает ключ со значением в построенной карте. Повторяющиеся ключи не допускаются и приведут к сбою build().

Не используйте повторно один и тот же экземпляр ImmutableMap.Builder.

Вместо этого вы можете клонировать объект вроде этого:

public ClientKey(ClientKey copyee) {
    // Copy fields here
    this.parameterMap = ImmutableMap.copyOf(copyee.parameterMap);
}

Если вы хотите использовать какой-то объект строителя, вы можете сделать что-то вроде этого:

public Builder(ClientKey copyee) {
    this.oldParameterMap = copyee.parameterMap;
}

public ClientKey build() {
    // Create new map here and pass it to new ClientKey somehow
    ImmutableMap.copyOf(oldParameterMap);
    return newKey;
}
person errantlinguist    schedule 17.12.2016
comment
Да, я смог понять, почему он выдает исключение, но как это исправить в моем случае? - person ; 17.12.2016
comment
Опять же, не используйте повторно один и тот же экземпляр ImmutableMap.Builder. - person errantlinguist; 17.12.2016
comment
Можете ли вы привести пример отказа от повторного использования одного и того же экземпляра, чтобы я мог это понять? - person ; 17.12.2016
comment
Я добавил небольшой пример, чтобы помочь вам понять немного лучше. - person errantlinguist; 17.12.2016
comment
Я использовал этот пример, который демонстрирует, как клонировать шаблон построителя из старого в новый. В нем упоминалось о передаче старой ссылки в конструкторе Builder. - person ; 17.12.2016
comment
Я до сих пор не понимаю, зачем нужен дополнительный класс строителя только для копирования объекта; У вас есть причина или вы просто делаете это, потому что видели это там? - person errantlinguist; 17.12.2016
comment
Я добавил пример с каким-то объектом-строителем. - person errantlinguist; 17.12.2016
comment
Ваш new ImmutableMap.Builder().putAll(...).build(); ничем не лучше, чем ImmutableMap.copyOf(oldParameterMap). - person maaartinus; 18.12.2016