debugjavaMinor
Map error messages to conditions
Viewed 0 times
maperrormessagesconditions
Problem
I have a method which checks some preconditions and if everything is fine returns a
My problem is that I have a bunch of
Here is an example code:
And here is my response:
Those conditions are getting more and more, and do not like these bunch of
success response object, otherwise fail with an appropriate error message.My problem is that I have a bunch of
if-elses which are checking some specific condition and if it fails return a fail object with a message.Here is an example code:
public Response sendFirstInquiryEmail(long itemId) {
Item item = itemService.findById(itemId);
if (item == null) {
return Response.fail().withComment("Can't find item");
}
if (State.NEW != item.getState()) {
return Response.fail().withComment("Can't send first inquiry to customer for items not in 'NEW' state");
}
if (!item.hasNumber()) {
return Response.fail().withComment("Can't find ticket in OTRS");
}
if (!item.isActive()) {
return Response.fail().withComment("Item is not active anymore");
}
if (!item.isReady()) {
return Response.fail().withComment("Item is not finalized, first inquiry can be send only for processed items");
}
send(item);
return Response.success();
}And here is my response:
public class Response {
private String comment;
private final boolean successful;
private Response(boolean successful) {
this.successful = successful;
}
public static Response success() {
return new Response(true);
}
public static Response fail() {
return new Response(false);
}
Response withComment(String comment) {
this.comment = comment;
return this;
}
public String comment() {
return comment;
}
public boolean isSuccessful() {
return successful;
}
}Those conditions are getting more and more, and do not like these bunch of
if. Is there a better way to achieve the same? I was thinking if it is possible somehow to map this conditioSolution
I am going to critique your
I would create a constant
With the above class, any constructed Response is automatically a fail... only the static
Your use code then becomes:
As for all those conditions that are tested in the cascading
I would live with it. It is clear, flows logically, and so on.
Response class. Firstly, I think that code should handle the common cases really well, and, hopefully, success happens often.I would create a constant
SUCCESS instance to indicate the success, and all other instances are fails. As a consequence, I would not have the withComment method. How about:public final class Response {
public static final Response SUCCESS = new Response();
private final String comment;
private Response() {
this.comment = null;
}
public Response (String comment) {
this.comment = Objects.requireNonNull(comment, "Fail responses require a non-null comment");
}
public String comment() {
return comment;
}
public boolean isSuccessful() {
return this == SUCCESS;
}
}With the above class, any constructed Response is automatically a fail... only the static
SUCCESS instance is a success.Your use code then becomes:
public Response sendFirstInquiryEmail(long itemId) {
Item item = itemService.findById(itemId);
if (item == null) {
return new Response("Can't find item");
}
if (State.NEW != item.getState()) {
return new Response("Can't send first inquiry to customer for items not in 'NEW' state");
}
if (!item.hasNumber()) {
return new Response("Can't find ticket in OTRS");
}
if (!item.isActive()) {
return new Response("Item is not active anymore");
}
if (!item.isReady()) {
return new Response("Item is not finalized, first inquiry can be send only for processed items");
}
send(item);
return Response.SUCCESS;
}As for all those conditions that are tested in the cascading
if blocks? Well, there's not much you can do about that. They are guard conditions. Your only option is to perhaps incorporate the cascade somewhere else, but all that does is shift the logic.I would live with it. It is clear, flows logically, and so on.
Code Snippets
public final class Response {
public static final Response SUCCESS = new Response();
private final String comment;
private Response() {
this.comment = null;
}
public Response (String comment) {
this.comment = Objects.requireNonNull(comment, "Fail responses require a non-null comment");
}
public String comment() {
return comment;
}
public boolean isSuccessful() {
return this == SUCCESS;
}
}public Response sendFirstInquiryEmail(long itemId) {
Item item = itemService.findById(itemId);
if (item == null) {
return new Response("Can't find item");
}
if (State.NEW != item.getState()) {
return new Response("Can't send first inquiry to customer for items not in 'NEW' state");
}
if (!item.hasNumber()) {
return new Response("Can't find ticket in OTRS");
}
if (!item.isActive()) {
return new Response("Item is not active anymore");
}
if (!item.isReady()) {
return new Response("Item is not finalized, first inquiry can be send only for processed items");
}
send(item);
return Response.SUCCESS;
}Context
StackExchange Code Review Q#91281, answer score: 3
Revisions (0)
No revisions yet.