HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMinor

Single class which holds response and error message

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
errorresponsemessageandsinglewhichholdsclass

Problem

I am working on a library which will make HTTP call to my rest service basis on inputs passed to this library. And whatever response comes back from service whether it is successful response or failure coming from the service or something happened in this library like timeout or any other errors, I am returning a response object back to the customer which customer can look into and iterate the object accordingly.

My response object is like this as of now and this is what I am returning back to the customer and customer will use this object and iterate it to figure out whether it is successful response or some failure and use it accordingly.

public class DataResponse {

    // response data
    private final String response;
    private final String date;
    private final String confidence;
    private final String mask;
    // what are the errors?
    private final ErrorCode error;
    // whether it is successful response or not
    private final StatusCode status;

    public DataResponse(String response, String date, String confidence, String mask, ErrorCode error, StatusCode status) {
        this.response = response;
        this.date = date;
        this.confidence = confidence;
        this.mask = mask;
        this.error = error;
        this.status = status;
    }

    // getters here

}


And below is my ErrorCode class which contains all the errors happened at client level or service level so that the customer can know what happened.

```
public enum ErrorCode {

OK(200, "NONE", "Response is success."),
NO_CONTENT(204, "No Content", "Response is success."),
CLIENT_TIMEOUT(1007, "Timeout", "Timeout has occured on the Client.");
// some more error messdates, keeping it short to show the idea

private final int code;
private final String status;
private final String description;

private ErrorCode(int code, String status, String description) {
this.code = code;
this.status = status;
this.desc

Solution

Is that error, or status, or both, or neither?

This API is a bit confusing.
You have ErrorCode and StatusCode enums. But ErrorCode contains things like:

OK(200, "NONE", "Response is success."),
NO_CONTENT(204, "No Content", "Response is success."),
CLIENT_TIMEOUT(1007, "Timeout", "Timeout has occured on the Client.");


So two of the "error codes" indicate success.

public enum StatusCode {
    SUCCESS, ERROR;
}


... And StatusCode can indicate success or error.

... The confusing nature of these elements make it difficult to understand the API, and the distinction of its building blocks.

Your error codes resemble HTTP status codes. Why not go a bit further in following HTTP practices, and have just one Status enum, that contains status codes like in the HTTP protocol, some of which are success, and others are error.

DataResponse

I like that the class is immutable.
But I'm concerned about the long parameter list of the constructor,
especially considering that the first 4 values are all of String type,
which can lead to errors such as mistaken ordering.

I would suggest to consider adding a builder,
that will have well-named setters to set the parameter values,
possibly allowing some sensible defaults.

Lastly, the type of the date field is a String.
It would be good to add a JavaDoc about the required format.
(For that matter, a JavaDoc for the other String parameters would be good too.)

Code Snippets

OK(200, "NONE", "Response is success."),
NO_CONTENT(204, "No Content", "Response is success."),
CLIENT_TIMEOUT(1007, "Timeout", "Timeout has occured on the Client.");
public enum StatusCode {
    SUCCESS, ERROR;
}

Context

StackExchange Code Review Q#114087, answer score: 5

Revisions (0)

No revisions yet.