如何克隆旧构建器以创建新的构建器对象？

Question

我有一个构建器类，我在我的一个项目中使用它。

• 假设我有 metricA 作为基于以下类的构建器。
• 我需要在metricA 的基础上通过克隆metricA 来创建一个新的构建器metricB，以便metricB 包含metricA 中已经存在的所有值。

在MetricHolder的构造函数中，我正在根据已经设置的字段初始化一些字段（不是直接设置的）。

• clientTypeOrPayId - 我正在初始化这个字段。如果payId 存在，那么我将设置此值或设置clientType。
• clientKey - 我也在同一个构造函数中初始化这个字段。
• 最重要的是，我在clientPayload 映射中添加了一些必填字段。我不确定这样做的正确方法是什么。但我需要将is_clientid 和is_deviceid 添加到地图中。 （总的来说，我会添加更多字段）。
• 然后在最后一个构造函数中，我正在计算延迟差异并将其发送到其他系统。

下面是我的课：

public final class MetricHolder {
  private final String clientId;
  private final String deviceId;
  private final String payId;
  private final String clientType;
  private final String clientTypeOrPayId;
  private final Schema schema;
  private final String schemaId;
  private final String clientKey;
  private final Map<String, String> clientPayload;
  private final Record record;
  private final long clientCreateTimestamp;
  private final long clientSentTimestamp;

  private MetricHolder(Builder builder) {
    this.payId = builder.payId;
    this.siteId = builder.siteId;
    this.clientType = builder.clientType;
    this.clientId = builder.clientId;
    this.deviceId = builder.deviceId;
    this.schema = builder.schema;
    this.schemaId = builder.schemaId;
    // populating all the required fields in the map and make it immutable
    // not sure whether this is right?
    builder.clientPayload.put("is_clientid", (clientId == null) ? "false" : "true");
    builder.clientPayload.put("is_deviceid", (clientId == null) ? "true" : "false");
    this.clientPayload = Collections.unmodifiableMap(builder.clientPayload);
    this.clientTypeOrPayId = Strings.isNullOrEmpty(payId) ? clientType : payId;
    this.record = builder.record;
    this.clientKey = "process:" + System.currentTimeMillis() + ":"
                        + ((clientId == null) ? deviceId : clientId);
    this.clientCreateTimestamp = builder.clientCreateTimestamp;
    this.clientSentTimestamp = builder.clientSentTimestamp;
    // this will be called twice while cloning
    // what is the right way to do this then?
    SendData.getInstance().insert(clientTypeOrPayId,
        System.currentTimeMillis() - clientCreateTimestamp);
    SendData.getInstance().insert(clientTypeOrPayId,
        System.currentTimeMillis() - clientSentTimestamp);
  }

  public static class Builder {
    private final Record record;
    private Schema schema;
    private String schemaId;
    private String clientId;
    private String deviceId;
    private String payId;
    private String clientType;
    private Map<String, String> clientPayload;
    private long clientCreateTimestamp;
    private long clientSentTimestamp;

    // this is for cloning
    public Builder(MetricHolder packet) {
      this.record = packet.record;
      this.schema = packet.schema;
      this.schemaId = packet.schemaId;
      this.clientId = packet.clientId;
      this.deviceId = packet.deviceId;
      this.payId = packet.payId;
      this.clientType = packet.clientType;
      // make a new map and check whether mandatory fields are present already or not
      // and if they are present don't add it again.
      this.clientPayload = new HashMap<>();
      for (Map.Entry<String, String> entry : packet.clientPayload.entrySet()) {
        if (!("is_clientid".equals(entry.getKey()) || "is_deviceid".equals(entry.getKey())) {
          this.clientPayload.put(entry.getKey(), entry.getValue());
        }
      }
      this.clientCreateTimestamp = packet.clientCreateTimestamp;
      this.clientSentTimestamp = packet.clientSentTimestamp;
    }

    public Builder(Record record) {
      this.record = record;
    }

    public Builder setSchema(Schema schema) {
      this.schema = schema;
      return this;
    }

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

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

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

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

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

    public Builder setClientPayload(Map<String, String> payload) {
      this.clientPayload = payload;
      return this;
    }

    public Builder setClientCreateTimestamp(long clientCreateTimestamp) {
      this.clientCreateTimestamp = clientCreateTimestamp;
      return this;
    }

    public Builder setClientSentTimestamp(long clientSentTimestamp) {
      this.clientSentTimestamp = clientSentTimestamp;
      return this;
    }

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

    // getters
}

问题：-

以下是我如何制作metricA builder 对象：

MetricHolder metricA = new MetricHolder.Builder(record).setClientId("123456").setDeviceId("abcdefhg")
                .           setPayId("98765").setClientPayload(payloadMapHolder).setClientCreateTimestamp(createTimestamp)
                            .setClientSentTimestamp(sentTimestamp).build();

这就是我稍后在获取所有其他字段时在代码中克隆 metricA 对象的方式，如下所示：

MetricHolder metricB = new MetricHolder.Builder(metricA).setSchema(schema).setSchemaId("345").build();

我现在看到两个问题：

• 首先，MetricHolder 构造函数中的SendData.getInstance() 行将被调用两次。首先是当我制作metricA 时，其次是当我通过克隆metricA 制作metricB 时。但是当我尝试创建 metricA 构建器对象时，我只想调用它一次？我怎样才能做到这一点？
• 其次，我在MetricHolder 构造函数中使用两个必填字段填充clientPayload 映射的方式对我来说看起来不正确。有没有其他更好的方法来做同样的事情？

我想整个问题正在发生，因为我克隆metricA 以创建metricB 构建器对象的方式？做这个的最好方式是什么？我想以正确的方式实现上述两件事。

Answer 1

但我只想在尝试创建 metricA 构建器对象时调用它一次？我怎样才能做到这一点？

最直接的方法是在构建器中设置一个标志，指示它是由Record 创建还是通过克隆创建的：

class Builder {
  final boolean cloned;

  Builder(MetricHolder packet) {
    this.cloned = true;
    // ...
  }

  Builder(Record record) {
    this.cloned = false;
    // ...
  }
}

那么，在MetricHolder的构造函数中：

if (!builder.cloned) {
  SendData.getInstance().whatever();
}

但值得指出的是，调用SendData 是doing too much work in the constructor 的一个示例。您应该仔细考虑是否真的要在构造函数中进行此调用，或者是否可以将其分解到另一个方法中。

其次，我在 MetricHolder 构造函数中使用两个必填字段填充 clientPayload 映射的方式对我来说看起来不正确。有没有其他更好的方法来做同样的事情？

您误解了使用Collections.unmodifiableMap 的“不可修改”位：它只是地图参数的不可修改的视图；您仍然可以修改底层地图。

这是一个 JUnit 测试来演示：

Map<String, String> original = new HashMap<>();
original.put("hello", "world");

// Obviously false, we just put something into it.
assertFalse(original.isEmpty());

Map<String, String> unmodifiable = Collections.unmodifiableMap(original);
// We didn't modify the original, so we don't expect this to have changed.
assertFalse(original.isEmpty());
// We expect this to be the same as for the original.
assertFalse(unmodifiable.isEmpty());

try {
  unmodifiable.clear();
  fail("Expected this to fail, as it's unmodifiable");
} catch (UnsupportedOperationException expected) {}

// Yep, still the same contents.
assertFalse(original.isEmpty());
assertFalse(unmodifiable.isEmpty());

// But here's where it gets sticky - no exception is thrown.
original.clear();
// Yep, we expect this...
assertTrue(original.isEmpty());

// But - uh-oh - the unmodifiable map has changed!
assertTrue(unmodifiable.isEmpty());

问题是地图只有在没有其他引用时才不可修改：如果您没有引用original，unmodifiable 实际上是不可修改的；否则，你不能依赖地图永远不会改变。

在您的特定情况下，您只是将 clientPayload 映射包装在不可修改的集合中。因此，您将覆盖先前构造的实例的值。

例如：

MetricHolder.Builder builder = new MetricHolder.Builder();
MetricHolder first = builder.build();
assertEquals("false", first.clientPayload.get("is_clientid"));
assertEquals("true", first.clientPayload.get("is_deviceid"));

builder.setClientId("").build();
// Hmm, first has changed.
assertEquals("true", first.clientPayload.get("is_clientid"));
assertEquals("false", first.clientPayload.get("is_deviceid"));

正确的做法是不要包装builder.clientPayload。把地图复制一份，修改一下，然后用unmodifiableMap换行：

{
  Map<String, String> copyOfClientPayload = new HashMap<>(builder.clientPayload);
  copyOfClientPayload.put("is_clientid", (clientId == null) ? "false" : "true");
  copyOfClientPayload.put("is_deviceid", (clientId == null) ? "true" : "false");
  this.clientPayload = Collections.unmodifiableMap(copyOfClientPayload);
}

周围的{} 不是绝对必要的，但它们限制了copyOfClientPayload 的范围，因此您以后不会在构造函数中意外重用它。