[GitHub] [beam] lastomato commented on a change in pull request #12320: [BEAM-10512] adds parsedData member variable to HL7v2Message and a Builder for the class

[GitBox <gi...@apache.org>]

lastomato commented on a change in pull request #12320:
URL: https://github.com/apache/beam/pull/12320#discussion_r470226727



##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HL7v2Message.java
##########
@@ -52,16 +55,19 @@ public String toString() {
    * @return the hl7v2 message
    */
   public static HL7v2Message fromModel(Message msg) {
-    SchematizedData schematizedData = msg.getSchematizedData();
-    return new HL7v2Message(
-        msg.getName(),
-        msg.getMessageType(),
-        msg.getSendTime(),
-        msg.getCreateTime(),
-        msg.getData(),
-        msg.getSendFacility(),
-        schematizedData != null ? schematizedData.getData() : null,
-        msg.getLabels());
+    HL7v2MessageBuilder mb =
+        new HL7v2MessageBuilder(
+            msg.getName(),
+            msg.getMessageType(),
+            msg.getSendTime(),
+            msg.getCreateTime(),
+            msg.getData(),
+            msg.getSendFacility());
+    mb.parsedData = msg.getParsedData();

Review comment:
       nit: please use the setters

##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HL7v2Message.java
##########
@@ -77,28 +83,66 @@ public Message toModel() {
     out.setCreateTime(this.getCreateTime());
     out.setData(this.getData());
     out.setSendFacility(this.getSendFacility());
-    out.setSchematizedData(new SchematizedData().setData(this.schematizedData));
-    out.setLabels(this.labels);
+    if (this.getParsedData() != null) {
+      out.setParsedData(this.getParsedData());
+    }
+    if (this.getSchematizedData() != null) {
+      out.setSchematizedData(new SchematizedData().setData(this.schematizedData));
+    }
+    if (this.getLabels() != null) {
+      out.setLabels(this.labels);

Review comment:
       nit: the null checks here are redundant since the setters take nullable inputs.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org