JsonMergePatchHelper pattern can be simplified

[alzimmermsft]

While regenerating an SDK to use stream-style serialization I found that the JsonMergePatchHelper pattern used to turn on and off patch serialization doesn't work correctly for polymorphic types.

When the models support JSON merge patch each model will create an access helper and add a boolean flag to determine whether JSON merge patch serialization should be used when calling toJson. This isn't necessary and can be a single instance on the super type by having the super type expose the patch serialization status to child types. So instead of:

public class Super {

  static {
    // Configure access helper
  }

  private boolean jsonMergePatch;

  public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
    if (jsonMergePatch) {
    ...
  }
}

public class Child extends Super {

  static {
    // Configure access helper
  }

  private boolean jsonMergePatch;

  public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
    if (jsonMergePatch) {
    ...
  }
}

it can be simplified to

public class Super {

  static {
    // Configure access helper
  }

  private boolean jsonMergePatch;

  boolean isJsonMergePatch() {
    return jsonMergePatch;
  }

  public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
    if (isJsonMergePatch()) {
    ...
  }
}

public class Child extends Super {

  public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
    if (isJsonMergePatch()) {
    ...
  }
}

And since there is boilerplate code for each accessor this will simplify that to one instance per polymorphic model hierarchy.

Further investigation could be done to see if it is possible for all models within a single package could use the same access helper.

[haolingdong-msft]

Thanks Alan for the suggestion, it could be better. Could you explain more on below statement, as I don't quite get the issue with having jsonMergePatch flag for both parent and child classes.

This isn't necessary and can be a single instance on the super type by having the super type expose the patch serialization status to child types.

[alzimmermsft]

Thanks Alan for the suggestion, it could be better. Could you explain more on below statement, as I don't quite get the issue with having jsonMergePatch flag for both parent and child classes.

This isn't necessary and can be a single instance on the super type by having the super type expose the patch serialization status to child types.

There isn't an explicit issue, it'll work perfectly fine but a parent type and child types both defining a property result in larger models and a bit of confusion when looking at code (mainly around, does setting this update the parent property or the child property). And this can be simplified to just the parent model defining the value and allowing children to access it, and will remove the number of accessors that need to be generated (for example in what I'm trying to regenerate this will remove 20-30 accessors that are never used as they're child types).