2
\$\begingroup\$

I have created these builders to help creating fluent acceptance tests for my api.

I would really like suggestions for improvements because this is supposed to be a "how to" project on creating acceptance tests.

Questions: Could this be improved? Should asJson be catching the exceptions to avoid polluting it's users? Should the static factory methods be creating the default values or the builders? Do you like the naming conventions Builder, I feel like they are silly when there is no asJson method but they are still only there to be a part of creating json...

public class HolidayBuilder {
 @JsonProperty
 private String name;
 @JsonProperty
 private String date;
 @JsonProperty
 private String duration;
 public HolidayBuilder withName(String name) {
 this.name = name;
 return this;
 }
 public HolidayBuilder withDate(String date) {
 this.date = date;
 return this;
 }
 public HolidayBuilder withDuration(String duration) {
 this.duration = duration;
 return this;
 }
 public String asJson() {
 try {
 return new ObjectMapper().writeValueAsString(this);
 } catch (JsonProcessingException e) {
 throw new RuntimeException("Could not create json: " + e.getMessage());
 }
 }
 public static HolidayBuilder aHoliday() {
 return new HolidayBuilder().withDuration("1d");
 }
}
public class WorkAttributeBuilder {
 @JsonProperty
 private int id;
 @JsonProperty
 private String key;
 @JsonProperty
 private WorkAttributeTypeBuilder type;
 @JsonProperty
 private String name;
 public WorkAttributeBuilder() {
 type = aAttributeType();
 }
 public WorkAttributeBuilder withId(int id) {
 this.id = id;
 return this;
 }
 public WorkAttributeBuilder withKey(String key) {
 this.key = key;
 return this;
 }
 public WorkAttributeBuilder withoutKey() {
 this.key = null;
 return this;
 }
 public WorkAttributeBuilder withType(WorkAttributeTypeBuilder type) {
 this.type = type;
 return this;
 }
 public WorkAttributeBuilder withName(String name) {
 this.name = name;
 return this;
 }
 public String asJson() {
 try {
 return new ObjectMapper().writeValueAsString(this);
 } catch (JsonProcessingException e) {
 throw new RuntimeException("Could not create json: " + e.getMessage());
 }
 }
 public static WorkAttributeBuilder aWorkAttribute() {
 return new WorkAttributeBuilder().withName("Some Name");
 }
}
public class WorkAttributeTypeBuilder {
 @JsonProperty
 private String value;
 public WorkAttributeTypeBuilder(String value) {
 this.value = value;
 }
 public String getValue() {
 return value;
 }
 public static WorkAttributeTypeBuilder aAttributeType() {
 return new WorkAttributeTypeBuilder("ACCOUNT");
 }
}
public class WorkAttributeValueBuilder {
 @JsonProperty
 private long worklogId;
 @JsonProperty
 private String value;
 @JsonProperty
 private WorkAttributeBuilder workAttribute;
 public WorkAttributeValueBuilder withWorklogId(long worklogId) {
 this.worklogId = worklogId;
 return this;
 }
 public WorkAttributeValueBuilder withValue(String value) {
 this.value = value;
 return this;
 }
 public WorkAttributeValueBuilder withWorkAttribute(WorkAttributeBuilder workAttribute) {
 this.workAttribute = workAttribute;
 return this;
 }
 public String asJson() {
 try {
 return new ObjectMapper().writeValueAsString(this);
 } catch (JsonProcessingException e) {
 throw new RuntimeException("Could not create json: " + e.getMessage());
 }
 }
 public static WorkAttributeValueBuilder aWorkAttributeValue() {
 return new WorkAttributeValueBuilder();
 }
}
asked Nov 11, 2016 at 9:38
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Your code is overall pretty nice. A few things I noted:

  • Upon first reading, I did a double-take at reading the withFoo methods that your fluent API uses. This is because there's an idiom where withFoo is sometimes a "setter" for an immutable object (which creates a new version with the update info). But after that, I read the contents of the methods and it made sense.

  • This is not the best way to rethrow exceptions:

    public String asJson() {
     try {
     return new ObjectMapper().writeValueAsString(this);
     } catch (JsonProcessingException e) {
     throw new RuntimeException("Could not create json: " + e.getMessage());
     }
    }
    

    Notice that RuntimeException has a constructor which takes a throwable reason as well. Thus it would be much better to implement this like so:

    public String asJson() {
     try {
     return new ObjectMapper().writeValueAsString(this);
     } catch (JsonProcessingException e) {
     throw new RuntimeException("Could not create json.", e);
     }
    }
    

    Don't worry; it still has the message from e. Note that your other classes also have this problem.

  • What's with the name of this method?

    public static HolidayBuilder aHoliday() {
     return new HolidayBuilder().withDuration("1d");
    }
    

    What does aHoliday mean? Your other classes have similarly named methods, and I don't understand what they are supposed to do. You should give them proper names.

answered Nov 11, 2016 at 13:11
\$\endgroup\$
2
  • \$\begingroup\$ I will probably be adding to the factory methods when many consumers need the same set up that could be named. The idea about aHoliday and the others is to create a builder already with some defaults that make thing in question valid. For the client aHoliday().withDate(date).toJson() is very readable. Maybe someHoliday is more readable? Do you have any suggestions on this? \$\endgroup\$ Commented Nov 11, 2016 at 18:01
  • \$\begingroup\$ @Jakob Oh so a was actually a word. "someHoliday" is much better then. On the other hand, the defaults should probably not be in the class, but in some "configuration settings" somewhere. \$\endgroup\$ Commented Nov 11, 2016 at 18:20

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.