12

I have a client library in which I am making http remote calls to my rest service and then I am returning List<DataResponse> back to the customer who is calling our library with the response I am getting from my REST service along with any errors if there are any wrapped around DataResponse object.

public class DataResponse {

    private final String response;
    private final boolean isLink;
    private final TypeOfId idType;
    private final long ctime;
    private final long lmd;
    private final String maskInfo;

    // below are for error stuff
    private final ErrorCode error;
    private final StatusCode status;

    // constructors and getters here

}

Here is my ErrorCode enum class:

public enum ErrorCode {

    // enum values

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

    // constructors and getters

}

And here is my StatusCode enum class:

public enum StatusCode {
    SUCCESS, FAILURE;
}

As you can see in my DataResponse class I have lot of fields so basis on that I have a very long constructor and everytime when I make a DataResponse object I have a big line with new DataResponse(.......). In future I might have more fields but for now I only have these fields.

Is there any better way I can use to make a DataResponse object and then return back List<DataResponse> from my library?

john
  • 11,311
  • 40
  • 131
  • 251
  • 5
    You can use builder pattern. – YoungHobbit Dec 25 '15 at 18:36
  • http://stackoverflow.com/a/6395981/3885376 – ROMANIA_engineer Dec 25 '15 at 18:38
  • I wouldn't depend on a builder yet. You have lots of fields? Sounds like a job for decomposition. Check out [this answer](http://stackoverflow.com/questions/33784390/object-oriented-design-how-important-is-encapsulation-when-therere-lots-of-da/33785266#33785266). If you still feel you are passing too much data to the constructor afterwards, then you could use a builder pattern. Although proper decomposition usually does the trick. I find builders to be useful for optional parameters (to avoid telescoping constructors which supply default values, rather than avoiding lots of parameters) – Vince Dec 25 '15 at 18:39
  • BUilderPattern is best to avoid long constructors or constructors havong two many parameters. – Ashish Ani Dec 25 '15 at 18:45
  • As too many parameters in constructors reduces readability of the code. – Ashish Ani Dec 25 '15 at 18:45
  • @VinceEmigh I might have some optional parameters here as well and some mandatory parameters. – john Dec 25 '15 at 18:45
  • @david If you have some optional and some mandatory parameter then you must have to use builder pattern – Ashish Ani Dec 25 '15 at 18:46
  • Adding a builder pattern is a Java best practice. – Ashish Ani Dec 25 '15 at 18:47
  • @AshishAni No, it's not for long parameters. It's best for optional parameters. If there are lots of parameters, it should be decomposed, as thats a sign that the type may have low cohesion. – Vince Dec 25 '15 at 18:48
  • You can find a lot of info from this SO post http://stackoverflow.com/questions/328496/when-would-you-use-the-builder-pattern. – Tech Enthusiast Dec 25 '15 at 18:48
  • @VinceEmigh Given my current scenario how would I use decomposition here? – john Dec 25 '15 at 18:49
  • @VinceEmigh For long parameters we may have some mandatory and some no so mandatory parameters. We can use builder pattern – Ashish Ani Dec 25 '15 at 18:49
  • @VinceEmigh according to book Effective Java by Joshua Bloch builder pattern is best for these scenarios only. – Ashish Ani Dec 25 '15 at 18:50
  • @david Your identifiers are not descriptive enough for me to say. `lmd`, `ctime` and `maskinfo` have no meaning to me, but maybe they could be decomposed into a `ResponseDetails` object – Vince Dec 25 '15 at 18:51
  • 1
    @AshishAni I don't think you interpreted that chapter correctly. I have also read Effective Java, which uses nutrition info as an example for builders - some nutrition values are optional, thus the builder pattern was a good deign for that scenario – Vince Dec 25 '15 at 18:52
  • Repetitive discussion. Check here http://stackoverflow.com/questions/7302891/the-builder-pattern-and-a-large-number-of-mandatory-parameters and here http://stackoverflow.com/questions/1638722/how-to-improve-the-builder-pattern – Tech Enthusiast Dec 25 '15 at 18:58
  • 1
    @YoungHobbit Read my answer. It gives details as to why a builder pattern is bad for "lots of constructor parameters". I'm surprised so many people misunderstand the purpose of the pattern. – Vince Dec 25 '15 at 19:13

3 Answers3

32

Do not use the builder pattern right away. It is not for types with tons of required fields. It's for types with tons of optional fields.

Builders' required properties are specified via the constructor. You are not forced to define values using methods, which makes those values optional.

This leaves potential for your object being only partially constructed. Using a builder for this would be abuse of the design.


With that said, you should decompose your type. I'm not sure what lmd or ctime is, or really what a DataResponse is supposed to represent, so I cannot tell you in which way you should decompose. But I can tell you cohesion is what determines such.

isLink, maskInfo and idType could possibly be decomposed into a DataResponseDetails object:

class DataResponseDetails {
    private boolean isLink;
    private String maskInfo;
    private TypeOfId idType;

    public DataResponseDetails(boolean isLink, String maskInfo, TypeOfId idType) {
        //...
    }
}

Now your DataResponse could be composed of DataResponseDetails:

class DataResponse {
    private DataResponseDetails details;
    private String response;
    //...

    public DataResponse(DataResponseDetails details, String response, ...) {
        //...
    }
}

Feel your constructor requires too much still? Decompose more!

Vince
  • 14,470
  • 7
  • 39
  • 84
  • I would add that, to me, it should be considered a best practice to use Optional class (the one from Guava or Java 8) for optional fields, to get rid of null values – Louis F. Dec 25 '15 at 19:36
4

Maybe you can identify smaller logical groups of fields an move them into objects of an own class. Then you can assemble all these objects in your DataResponse objects.

J. Su.
  • 451
  • 1
  • 6
  • 18
0

As Joshua Bloch stated it in Item 2 of Effective Java 2nd Edition, you should consider using a builder pattern, as it is a best practice.

Here is what you code could look like using it :

 public class DataResponse {

        private final String response;
        private final boolean isLink;
        private final TypeOfId idType;
        private final long ctime;
        private final long lmd;
        private final String maskInfo;

        // below are for error stuff
        private final ErrorCode error;
        private final StatusCode status;

        // constructors and getters here


        public static class Builder {

            private final String response;
            private final boolean isLink;
            private final TypeOfId idType;
            private final long ctime;
            private final long lmd;
            private final String maskInfo;

            // below are for error stuff
            private final ErrorCode error;
            private final StatusCode status;

            public Builder reponse(final String response) {
                this.response = response;
                return this;
            }

            public Builder isLing(final boolean isLink) {
                this.isLink = isLink;
                return this;
            }

            public DataResponse builder() {
                return new DataResponse(this);
            }

            ...

        }

        private DataResponse(final Builder builder) {
            this.response = builder.response;
          this.isLink  = builder.isLink;
        }
    }

and then do something as follow :

DataResponse response = new DataResponse.Builder().reponse(anyResponse).isLink(isLink).build();
Louis F.
  • 2,018
  • 19
  • 22