I'm looking for a better way to write my custom Exception
:
public class NoSuchTaskException extends BaseImpactException {
private final String taskId;
private final String taskType;
private final Long userId;
private final DocumentType documentType;
private final Long documentId;
public NoSuchTaskException(String taskId, String taskType, Long userId,
DocumentType documentType, Long documentId) {
this(taskType, userId, documentType, documentId, taskId, null);
}
public NoSuchTaskException(String taskType, Long userId, DocumentType documentType,
Long documentId) {
this(taskType, userId, documentType, documentId, null, null);
}
public NoSuchTaskException(String taskType, Long userId, DocumentType documentType,
Long documentId, String taskId, Throwable throwable) {
super("exception.task.noSuchTask", throwable);
this.taskId = taskId;
this.taskType = taskType;
this.userId = userId;
this.documentType = documentType;
this.documentId = documentId;
}
// ...
}
which I then use in various places:
throw new NoSuchTaskException(taskId, null, isUserPresent ? null : addedById, null, null)
;throw new NoSuchTaskException(taskId, taskType.name(), taskExecutorId, null, null)
;throw new NoSuchTaskException(taskId, null, null, null, null)
, ...I think from the usage examples, it is clear why I think this might not be the best implementation/usage. My idea was to improve this by utilizing a builder pattern:
public class NoSuchTaskException extends BaseImpactException {
private final String taskId;
private final String taskType;
private final Long userId;
private final DocumentType documentType;
private final Long documentId;
private NoSuchTaskException(Builder builder) {
super("exception.task.noSuchTask", builder.throwable);
this.taskId = builder.taskId;
this.taskType = builder.taskType;
this.userId = builder.userId;
this.documentType = builder.documentType;
this.documentId = builder.documentId;
}
public static Builder builder(String taskId) {
return new Builder(taskId);
}
@RequiredArgsConstructor
public static class Builder {
private String taskId;
private String taskType;
private Long userId;
private DocumentType documentType;
private Long documentId;
private Throwable throwable;
public Builder taskId(String taskId) {
this.taskId = taskId;
return this;
}
public Builder taskType(String taskType) {
this.taskType = taskType;
return this;
}
public Builder userId(Long userId) {
this.userId = userId;
return this;
}
public Builder documentType(DocumentType documentType) {
this.documentType = documentType;
return this;
}
public Builder documentId(Long documentId) {
this.documentId = documentId;
return this;
}
public Builder throwable(Throwable throwable) {
this.throwable = throwable;
return this;
}
public NoSuchTaskException build() {
return new NoSuchTaskException(this);
}
}
}
which I can then use as:
throw NoSuchTaskException.builder("task123")
.documentType(DocumentType.DOCUMENT_B)
.documentId(500L)
.build();
instead of using it like this:
throw new NoSuchTaskException("task123", null, null, DocumentType.DOCUMENT_B, 500L);
Is this an acceptable approach? Should I use the builder pattern in my Exception
, or is it an overkill (or even a code smell)?
You are already using lombok, so... just use lombok's builder then. Muuch simpler:
import lombok.Builder;
import lombok.NonNull;
import lombok.Value;
@Value
public class NoSuchTaskException extends BaseImpactException {
String taskId;
String taskType;
Long userId;
DocumentType documentType;
Long documentId;
@Builder
private NoSuchTaskException(@NonNull String taskId, String taskType, Long userId, DocumentType documentType, Long documentId, Throwable cause) {
super("exception.task.noSuchTask", cause);
this.taskId = builder.taskId;
this.taskType = builder.taskType;
this.userId = builder.userId;
this.documentType = builder.documentType;
this.documentId = builder.documentId;
}
}
One small 'trick' being used here is that Builder
has been put on the constructor; you can't put it on the type itself because you're extending something and lombok cannot divine that you'd want e.g. Throwable cause
but not String message
from your exception supertypes.
NB: @Value
makes all fields private
and final
(and even makes your class final
), and, of course, gives you getters for all of it and sensible toString, equals, and hashCode implementations.