2
\$\begingroup\$

This is a follow on question from one I asked a few days ago here: Single process blocking queue

Using the comments I determined that there probably isn't a pre-built solution for this and so I have had a go at writing my own implementation using a PriorityBlockingQueue.

In order to avoid propagation of poor code, before providing my implementation as an answer to my original question Are there any obvious or horrific flaws in my code! I know the first thing everybody is going to do is curse me to hell for using a singleton approach... does anyone have any alternatives/improvements.

Abstract Task

The only important value here is the int priority that determines how important the task is!

public abstract class Task {
    public abstract String id();
    public abstract Class<?> callback();
    public abstract int priority();
    public abstract Sector Sector();
    public abstract byte[] payload();
}


Task Implementation

My preference is for an immutable builder design.

/**
 * Immutable implementation of {@link Task}.
 * <p>
 * Use builder to create immutable instances:
 * {@code ImmutableTask.builder()}.
 * Use static factory method to create immutable instances:
 * {@code ImmutableTask.of()}.
 */
@SuppressWarnings("all")
public final class ImmutableTask extends Task {
  private final String id;
  private final Class<?> callback;
  private final int priority;
  private final Sector Sector;
  private final byte[] payload;

  private ImmutableTask(
      String id,
      Class<?> callback,
      int priority,
      Sector Sector,
      byte[] payload) {
    this.id = Objects.requireNonNull(id);
    this.callback = Objects.requireNonNull(callback);
    this.priority = priority;
    this.Sector = Objects.requireNonNull(Sector);
    this.payload = Objects.requireNonNull(payload);
  }

  private ImmutableTask(ImmutableTask.Builder builder) {
    this.id = builder.id;
    this.callback = builder.callback;
    this.priority = builder.priority;
    this.Sector = builder.Sector;
    this.payload = builder.payload;
  }

  private ImmutableTask(
      ImmutableTask original,
      String id,
      Class<?> callback,
      int priority,
      Sector Sector,
      byte[] payload) {
    this.id = id;
    this.callback = callback;
    this.priority = priority;
    this.Sector = Sector;
    this.payload = payload;
  }

  /**
   * {@inheritDoc}
   * @return value of {@code id} attribute
   */
  @Override
  public String id() {
    return id;
  }

  /**
   * {@inheritDoc}
   * @return value of {@code callback} attribute
   */
  @Override
  public Class<?> callback() {
    return callback;
  }

  /**
   * {@inheritDoc}
   * @return value of {@code priority} attribute
   */
  @Override
  public int priority() {
    return priority;
  }

  /**
   * {@inheritDoc}
   * @return value of {@code Sector} attribute
   */
  @Override
  public Sector Sector() {
    return Sector;
  }

  /**
   * {@inheritDoc}
   * @return cloned {@code payload} array
   */
  @Override
  public byte[] payload() {
    return payload.clone();
  }

  /**
   * Copy current immutable object by setting value for {@link Task#id() id}.
   * Shallow reference equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value new value for id
   * @return modified copy of the {@code this} object
   */
  public final ImmutableTask withId(String value) {
    if (this.id == value) {
      return this;
    }
    String newValue = Objects.requireNonNull(value);
    return new ImmutableTask(this, newValue, this.callback, this.priority, this.Sector, this.payload);
  }

  /**
   * Copy current immutable object by setting value for {@link Task#callback() callback}.
   * Shallow reference equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value new value for callback
   * @return modified copy of the {@code this} object
   */
  public final ImmutableTask withCallback(Class<?> value) {
    if (this.callback == value) {
      return this;
    }
    Class<?> newValue = Objects.requireNonNull(value);
    return new ImmutableTask(this, this.id, newValue, this.priority, this.Sector, this.payload);
  }

  /**
   * Copy current immutable object by setting value for {@link Task#priority() priority}.
   * Value equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value new value for priority
   * @return modified copy of the {@code this} object
   */
  public final ImmutableTask withPriority(int value) {
    if (this.priority == value) {
      return this;
    }
    int newValue = value;
    return new ImmutableTask(this, this.id, this.callback, newValue, this.Sector, this.payload);
  }

  /**
   * Copy current immutable object by setting value for {@link Task#Sector() Sector}.
   * Shallow reference equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value new value for Sector
   * @return modified copy of the {@code this} object
   */
  public final ImmutableTask withSector(com.solid.halo.representation.Sector value) {
    if (this.Sector == value) {
      return this;
    }
    com.solid.halo.representation.Sector newValue = Objects.requireNonNull(value);
    return new ImmutableTask(this, this.id, this.callback, this.priority, newValue, this.payload);
  }

  /**
   * Copy current immutable object with elements that replace content of {@link Task#payload() payload}.
   * Array is cloned before saved as the attribute value.
   * @param elements elements for payload, not null
   * @return modified copy of {@code this} object
   */
  public final ImmutableTask withPayload(byte... elements) {
    byte[] newValue = elements.clone();
    return new ImmutableTask(this, this.id, this.callback, this.priority, this.Sector, newValue);
  }

  /**
   * This instance is equal to instances of {@code ImmutableTask} with equal attribute values.
   * @return {@code true} if {@code this} is equal to {@code another} instance
   */
  @Override
  public boolean equals(Object another) {
    return this == another
        || (another instanceof ImmutableTask && equalTo((ImmutableTask) another));
  }

  private boolean equalTo(ImmutableTask another) {
    return id.equals(another.id)
        && callback.equals(another.callback)
        && priority == another.priority
        && Sector.equals(another.Sector)
        && Arrays.equals(payload, another.payload);
  }

  /**
   * Computes hash code from attributes: {@code id}, {@code callback}, {@code priority}, {@code Sector}, {@code payload}.
   * @return hashCode value
   */
  @Override
  public int hashCode() {
    int h = 31;
    h = h * 17 + id.hashCode();
    h = h * 17 + callback.hashCode();
    h = h * 17 + priority;
    h = h * 17 + Sector.hashCode();
    h = h * 17 + Arrays.hashCode(payload);
    return h;
  }

  /**
   * Prints immutable value {@code Task{...}} with attribute values,
   * excluding any non-generated and auxiliary attributes.
   * @return string representation of value
   */
  @Override
  public String toString() {
    return new StringBuilder("Task{")
        .append("id=").append(id)
        .append(", callback=").append(callback)
        .append(", priority=").append(priority)
        .append(", Sector=").append(Sector)
        .append(", payload=").append(Arrays.toString(payload))
        .append('}').toString();
  }

  /**
   * Construct new immutable {@code Task} instance.
   * @param id value for {@code id}
   * @param callback value for {@code callback}
   * @param priority value for {@code priority}
   * @param Sector value for {@code Sector}
   * @param payload value for {@code payload}
   * @return immutable Task instance
   */
  public static ImmutableTask of(String id, Class<?> callback, int priority, Sector Sector, byte[] payload) {
    return new ImmutableTask(id, callback, priority, Sector, payload);
  }

  /**
   * Creates immutable copy of {@link Task}.
   * Uses accessors to get values to initialize immutable instance.
   * If an instance is already immutable, it is returned as is.
   * @return copied immutable Task instance
   */
  public static ImmutableTask copyOf(Task instance) {
    if (instance instanceof ImmutableTask) {
      return (ImmutableTask) instance;
    }
    return ImmutableTask.builder()
        .from(instance)
        .build();
  }

  /**
   * Creates builder for {@link ImmutableTask}.
   * @return new ImmutableTask builder
   */
  public static ImmutableTask.Builder builder() {
    return new ImmutableTask.Builder();
  }

  /**
   * Builds instances of {@link ImmutableTask}.
   * Initialized attributes and then invoke {@link #build()} method to create
   * immutable instance.
   * <p><em>Builder is not thread safe and generally should not be stored in field or collection,
   * but used immediately to create instances.</em>
   */
  public static final class Builder {
    private static final long INITIALIZED_BITSET_ALL = 0x1f;
    private static final long INITIALIZED_BIT_ID = 0x1L;
    private static final long INITIALIZED_BIT_CALLBACK = 0x2L;
    private static final long INITIALIZED_BIT_PRIORITY = 0x4L;
    private static final long INITIALIZED_BIT_SECTOR = 0x8L;
    private static final long INITIALIZED_BIT_PAYLOAD = 0x10L;
    private long initializedBitset;

    private String id;
    private Class<?> callback;
    private int priority;
    private Sector Sector;
    private byte[] payload;
    private Builder() {}

    /**
     * Adjust builder with values from provided {@link Task} instance.
     * Regular attribute values will be overridden, i.e. replaced with ones of an instance.
     * Instance's absent optional values will not be copied (will not override current).
     * Collection elements and entries will be added, not replaced.
     * @param instance instance to copy values from
     * @return {@code this} builder for chained invocation
     */
    public final Builder from(Task instance) {
      Objects.requireNonNull(instance);
      id(instance.id());
      callback(instance.callback());
      priority(instance.priority());
      Sector(instance.Sector());
      payload(instance.payload());
      return this;
    }

    /**
     * Initializes value for {@link Task#id() id}.
     * @param id value for id
     * @return {@code this} builder for chained invocation
     */
    public final Builder id(String id) {
      this.id = Objects.requireNonNull(id);
      initializedBitset |= INITIALIZED_BIT_ID;
      return this;
    }

    /**
     * Initializes value for {@link Task#callback() callback}.
     * @param callback value for callback
     * @return {@code this} builder for chained invocation
     */
    public final Builder callback(Class<?> callback) {
      this.callback = Objects.requireNonNull(callback);
      initializedBitset |= INITIALIZED_BIT_CALLBACK;
      return this;
    }

    /**
     * Initializes value for {@link Task#priority() priority}.
     * @param priority value for priority
     * @return {@code this} builder for chained invocation
     */
    public final Builder priority(int priority) {
      this.priority = priority;
      initializedBitset |= INITIALIZED_BIT_PRIORITY;
      return this;
    }

    /**
     * Initializes value for {@link Task#Sector() Sector}.
     * @param Sector value for Sector
     * @return {@code this} builder for chained invocation
     */
    public final Builder Sector(Sector Sector) {
      this.Sector = Objects.requireNonNull(Sector);
      initializedBitset |= INITIALIZED_BIT_SECTOR;
      return this;
    }

    /**
     * Initializes value for {@link Task#payload() payload}.
     * @param elements elements for payload
     * @return {@code this} builder for chained invocation
     */
    public final Builder payload(byte... elements) {
      this.payload = elements.clone();
      initializedBitset |= INITIALIZED_BIT_PAYLOAD;
      return this;
    }

    /**
     * Builds new {@link ImmutableTask}.
     * @return immutable instance of Task
     */
    public ImmutableTask build() {
      checkRequiredAttributes();
      return new ImmutableTask(this);
    }

    private boolean idIsSet() {
      return (initializedBitset & INITIALIZED_BIT_ID) != 0;
    }

    private boolean callbackIsSet() {
      return (initializedBitset & INITIALIZED_BIT_CALLBACK) != 0;
    }

    private boolean priorityIsSet() {
      return (initializedBitset & INITIALIZED_BIT_PRIORITY) != 0;
    }

    private boolean SectorIsSet() {
      return (initializedBitset & INITIALIZED_BIT_SECTOR) != 0;
    }

    private boolean payloadIsSet() {
      return (initializedBitset & INITIALIZED_BIT_PAYLOAD) != 0;
    }

    private void checkRequiredAttributes() {
      if (initializedBitset != INITIALIZED_BITSET_ALL) {
        throw new IllegalStateException(formatRequiredAttributesMessage());
      }
    }

    private String formatRequiredAttributesMessage() {
      Collection<String> attributes = new ArrayList<String>();
      if (!idIsSet()) {
        attributes.add("id");
      }
      if (!callbackIsSet()) {
        attributes.add("callback");
      }
      if (!priorityIsSet()) {
        attributes.add("priority");
      }
      if (!SectorIsSet()) {
        attributes.add("Sector");
      }
      if (!payloadIsSet()) {
        attributes.add("payload");
      }
      return "Cannot build Task, some of required attributes are not set " + attributes;
    }
  }
}


Abstract Queue

The @Value.Default simply allows immutable implementations to be assigned default values. Using it with execTask() is a bit of a hack to start the execution thread during the singleton instantiation.

public abstract class Queue {

    @Value.Default
    protected int queueSize() {
        return 100;
    };

    @Value.Default
    protected Comparator<Task> compare() {
        return new Comparator<Task>() {
            public int compare(Task o1, Task o2) {
                return (o1.priority() > o2.priority() ? -1 : 1);
            }
        };
    };

    @Value.Default
    protected PriorityBlockingQueue<Task> taskQueue() {
        return new PriorityBlockingQueue<Task>(queueSize(), compare());
    };

    @Value.Default
    protected TaskExec taskExec() {
        return ImmutableTaskExec.builder().build();
    };

    /**
     * Takes the highest priority task from the queue and passes it to the task
     * executor. The task is removed from the list.
     */
    @Value.Default
    private synchronized void execTask() {
        // Create a new thread to run the consumer
        Runnable exec = new Runnable() {

            @Override
            public void run() {
                // Put the thread into a perpetual loop
                while (true) {
                    try {
                        // Take the task from the queue
                        Task task = taskQueue().take();
                        // Access the Task Executor
                        taskExec().execute(task);
                        // Throttle the loop
                        Thread.sleep(500);
                    } catch (InterruptedException e) {
                        // TODO log
                    }
                }
            }
        };
        new Thread(exec, "QUEUE_EXEC_THREAD").start();

    }

    /**
     * Adds a new task to the queue. The task order is prioritized based on the
     * comparator specified
     * 
     * @param task
     *            The task to be added to the queue
     * @throws ArrayStoreException
     *             If a task tries to be added and the queue is full
     */
    public final void addTask(Task task) throws ArrayStoreException {
        if (taskQueue().size() >= queueSize()) {
            // TODO log
            // TODO create custom exception
            throw new ArrayStoreException("QUEUE FULL");
        }
        try {
            taskQueue().put(task);
        } catch (Throwable e) {
            // TODO log
        }
    }

}


Queue Implementation

Again my preference is for an immutable design. I know singletons are considered bad design but I feel this is the best approach unless there is a better suggestion.

/**
 * Immutable implementation of {@link Queue}.
 * <p>
 * Use static factory method to get ault singleton instance:
 * {@code ImmutableQueue.of()}.
 */
@SuppressWarnings("all")
public final class ImmutableQueue extends Queue {
  private final int queueSize;
  private final Comparator<Task> compare;
  private final PriorityBlockingQueue<Task> taskQueue;
  private final TaskExec taskExec;

  private ImmutableQueue() {
    this.queueSize = super.queueSize();
    this.compare = Objects.requireNonNull(super.compare());
    this.taskQueue = Objects.requireNonNull(super.taskQueue());
    this.taskExec = Objects.requireNonNull(super.taskExec());
  }

  private ImmutableQueue(
      ImmutableQueue original,
      int queueSize,
      Comparator<Task> compare,
      PriorityBlockingQueue<Task> taskQueue,
      TaskExec taskExec) {
    this.queueSize = queueSize;
    this.compare = compare;
    this.taskQueue = taskQueue;
    this.taskExec = taskExec;
  }

  /**
   * {@inheritDoc}
   * @return value of {@code queueSize} attribute
   */
  @Override
  public int queueSize() {
    return queueSize;
  }

  /**
   * {@inheritDoc}
   * @return value of {@code compare} attribute
   */
  @Override
  public Comparator<Task> compare() {
    return compare;
  }

  /**
   * {@inheritDoc}
   * @return value of {@code taskQueue} attribute
   */
  @Override
  public PriorityBlockingQueue<Task> taskQueue() {
    return taskQueue;
  }

  /**
   * {@inheritDoc}
   * @return value of {@code taskExec} attribute
   */
  @Override
  public TaskExec taskExec() {
    return taskExec;
  }

  /**
   * Copy current immutable object by setting value for {@link Queue#queueSize() queueSize}.
   * Value equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value new value for queueSize
   * @return modified copy of the {@code this} object
   */
  public final ImmutableQueue withQueueSize(int value) {
    if (this.queueSize == value) {
      return this;
    }
    int newValue = value;
    return validate(new ImmutableQueue(this, newValue, this.compare, this.taskQueue, this.taskExec));
  }

  /**
   * Copy current immutable object by setting value for {@link Queue#compare() compare}.
   * Shallow reference equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value new value for compare
   * @return modified copy of the {@code this} object
   */
  public final ImmutableQueue withCompare(Comparator<Task> value) {
    if (this.compare == value) {
      return this;
    }
    Comparator<Task> newValue = Objects.requireNonNull(value);
    return validate(new ImmutableQueue(this, this.queueSize, newValue, this.taskQueue, this.taskExec));
  }

  /**
   * Copy current immutable object by setting value for {@link Queue#taskQueue() taskQueue}.
   * Shallow reference equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value new value for taskQueue
   * @return modified copy of the {@code this} object
   */
  public final ImmutableQueue withTaskQueue(PriorityBlockingQueue<Task> value) {
    if (this.taskQueue == value) {
      return this;
    }
    PriorityBlockingQueue<Task> newValue = Objects.requireNonNull(value);
    return validate(new ImmutableQueue(this, this.queueSize, this.compare, newValue, this.taskExec));
  }

  /**
   * Copy current immutable object by setting value for {@link Queue#taskExec() taskExec}.
   * Shallow reference equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value new value for taskExec
   * @return modified copy of the {@code this} object
   */
  public final ImmutableQueue withTaskExec(TaskExec value) {
    if (this.taskExec == value) {
      return this;
    }
    TaskExec newValue = Objects.requireNonNull(value);
    return validate(new ImmutableQueue(this, this.queueSize, this.compare, this.taskQueue, newValue));
  }

  /**
   * This instance is equal to instances of {@code ImmutableQueue} with equal attribute values.
   * @return {@code true} if {@code this} is equal to {@code another} instance
   */
  @Override
  public boolean equals(Object another) {
    return this == another
        || (another instanceof ImmutableQueue && equalTo((ImmutableQueue) another));
  }

  private boolean equalTo(ImmutableQueue another) {
    return queueSize == another.queueSize
        && compare.equals(another.compare)
        && taskQueue.equals(another.taskQueue)
        && taskExec.equals(another.taskExec);
  }

  /**
   * Computes hash code from attributes: {@code queueSize}, {@code compare}, {@code taskQueue}, {@code taskExec}.
   * @return hashCode value
   */
  @Override
  public int hashCode() {
    int h = 31;
    h = h * 17 + queueSize;
    h = h * 17 + compare.hashCode();
    h = h * 17 + taskQueue.hashCode();
    h = h * 17 + taskExec.hashCode();
    return h;
  }

  /**
   * Prints immutable value {@code Queue{...}} with attribute values,
   * excluding any non-generated and auxiliary attributes.
   * @return string representation of value
   */
  @Override
  public String toString() {
    return new StringBuilder("Queue{")
        .append("queueSize=").append(queueSize)
        .append(", compare=").append(compare)
        .append(", taskQueue=").append(taskQueue)
        .append(", taskExec=").append(taskExec)
        .append('}').toString();
  }

  private static final ImmutableQueue INSTANCE = validate(new ImmutableQueue());

  /**
   * Returns ault immutable singleton value of {@code Queue}
   * @return immutable instance of Queue
   */
  public static ImmutableQueue of() {
    return INSTANCE;
  }

  private static ImmutableQueue validate(ImmutableQueue instance) {
    return INSTANCE != null && INSTANCE.equalTo(instance) ? INSTANCE : instance;
  }
}


Abstract TaskExec

Doesn't actually do anything yet. However, this would be where the Task is actually transmitted to the hardware.

public abstract class TaskExec {

    public void execute(Task task) {
        System.out.println("Executed task " + task.toString());
        // TODO Send the task to the hardware
    }

}


Usage

Create + init the singleton Queue during application startup (in main()):

    ImmutableQueue.of();

Create and add a Task from any point in the application. Obviously passing in proper values for id, callback, priority, Sector and payload:

   Task task = ImmutableTask.of(id, callback, priority, Sector, payload);
   ImmutableQueue.of().addTask(task);

Thats it! technically the queue should then pass the highest priority Task in the queue to the TaskExec to execute.

\$\endgroup\$
2
\$\begingroup\$

Shouldn't class Task be an interface?

ImmutableTask

private ImmutableTask(
        String id,
        Class<?> callback,
        int priority,
        Sector Sector,
        byte[] payload) {
    this.id = Objects.requireNonNull(id);
    this.callback = Objects.requireNonNull(callback);
    this.priority = priority;
    this.Sector = Objects.requireNonNull(Sector);
    this.payload = Objects.requireNonNull(payload);
}

With storing the payload without cloning it first, the immutability says bye bye.

You're importing Sector, but using the FQN com.solid.halo.representation.Sector in withSector.

 @Override
 public byte[] payload() {
     return payload.clone();
 }

Consider calling it differently as it's the only getter doing something untypical.

 public final ImmutableTask withId(String value) {
     if (this.id == value) {
         return this;
     }
     final String newValue = Objects.requireNonNull(value);
     return new ImmutableTask(this, newValue, this.callback, this.priority, this.Sector, this.payload);
 }

You don't need newValue and I wouldn't bother with the this qualifier. Using simply

     Objects.requireNonNull(value);
     return new ImmutableTask(this, id, value, priority, Sector, payload);

allows you to copy&paste all the boilerplate more easily. It's also less error-prone as obviously everything is a field except for the provided argument and this itself....

... which you should remove. ImmutableTask original in the constructor doesn't get used, so why bother? Using it could lead to memory leaks as each instance would carry its whole history with it.

 /**
  * Computes hash code from attributes: {@code id}, {@code callback}, {@code priority}, {@code Sector}, {@code payload}.
  * @return hashCode value
  */

I wouldn't document it with so much detail. What if you want to change it later? It's good that -- unlike some JDK classes -- you don't document the exact formula as it'd make it hard to change it later. FYI, e.g., Map.Entry.hashCode is a pure crap, but once documented....

Actually, I wouldn't document it at all. What matters is equals and hashCode has to follow. It doesn't do anything special (like e.g. leaving fields out, which is permissible) so who cares?

 /**
  * Prints immutable value {@code Task{...}} with attribute values,
  * excluding any non-generated and auxiliary attributes.
  * @return string representation of value
  */
 @Override
 public String toString() {

That's not true. 1. It doesn't print anything. 2. The "non-generated" attributes don't get excluded. Maybe we could say that the "generated" do (there are none, so it's true). Again, I'd ditch it all or write

/** Returns a string representation of this. Note that it may change anytime. */

Providing both withers and a builder doesn't seem to be useful as they're pretty interchangeable. Sure, your builder allows you to make sure that no field gets forgotten, but so would a public constructor or a static factory method. Now I see, you have a factory method, too.

A builder is a huge advantage when there are many fields and many of them are of the same type. But you have 5 fields and each has a different type.

public final Builder from(Task instance) ...

I'd prefer a fluent syntax like task.toBuilder().

 public ImmutableTask build() {
     checkRequiredAttributes();
     return new ImmutableTask(this);
 }

This makes from unusable as you'll have to override all attributes (did you forgot initializedBitset |= INITIALIZED_BITSET_ALL; there?).

Summary

The class is fine (except for the non-cloning of incoming payload and the non-usability of Builder.from), but terribly verbose. Don't forget that writing code is not all, it will be read, again and again and then once more.

Because of this, I'm claiming that less is more.

Moreover, I'd strongly suggest to get a tool helping you with this. No code generator, as it wouldn't save you the reading. There's Autovalue and project Lombok (see my answer here).

I know Lombok well and if it works for you, it's a bless:

@RequiredArgsConstructor(access=AccessLevel.PRIVATE) @EqualsAndHashCode(callSuper=false) @Wither @Accessors(fluent=true, chain=true) @Getter

public final class ImmutableTask extends Task {
    @NonNull private final String id;
    @NonNull private final Class<?> callback;
    private final int priority;
    @NonNull private final Sector Sector;
    @NonNull private final byte[] payload;

    ... write `of(....)` which clones
    ... write `payload()` which clones
    ... write `withPayload()` which clones
    ... write `toBuilder`
}

So you need to write just 4 methods. Nice, isn't it? You lose a single feature, namely the check if all fields in the builder were set. There's a corresponding issue, but the Lombok authors have other things to do.

Everything else works out of the box, e.g., @EqualsAndHashCodeunderstands arrays and@Nonnull` adds the null checks to all constructors, withers, and setters.

Anyway, even with Lombok, I'd leave the builder out. If it's a public API, then it's hard to take anything out later. Adding is much easier. If it's for you, then work saved now may be needed later or not. Anyway, by not adding a dubious feature, you can only win.

Moreover, you want that all uses of the class look similar, right? And providing three different creation methods is a bad start.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.