5

I came across a DTO class like the below one.

class PersonDTO {
   private String firstName;
   private String middleName;
   private String lastName;
   private String dob;

   // some 50 fields more
   private List<PersonDTO> persons;

   //getters and setter
}

Question 1: Is it a good practice to have such huge DTO classes with so many properties?

Question 2: Is it a good practice to have a DTO class having an ArrayList of its own type? Wouldn't it cause a circular reference?

Update:

I work for a product in Healthcare domain. The use case is to generate a report of all the users in the system. Along with the list of users, the report also needs to show the summary information. The data need to be serialized, because the UI is expecting a JSON response.

Below is the actual DTO I was taking about.

A method was written in DAO class, which returns a UserDTO object. This UserDTO object that is being returned.. consists of a List of all users, and some summary information like: total doctors, total nurses etc.

class UserDTO{
    private String userID; //This is unique

    private String firstName;
    private String middleName;
    private String lastName;
    private String dob;

    private String userType; // value can be DT(for doctor), ST(for nurse), 

    private int totalDoctorCount; //Holds the total number of UserDTOs with userType value set to DT in the ArrayList of users.

    private int totalNurseCount; //Holds the total number of UserDTOs with userType value set to ST in the ArrayList of users.


    // there are some 40 more properties.

    private List<UserDTO> users;

        //In this class there are also properties like below... 

        private String primaryAddrStreetName;
        private String primaryAddrCity;
        private String primaryAddrState;
        private String primaryAddrCountry;
        private String primaryAddrZipcode;

        private String billingAddrStreetName;
        private String billingAddrCity;
        private String billingAddrState;
        private String billingAddrCountry;
        private String billingAddrZipcode;

        private String shippingAddrStreetName;
        private String shippingAddrCity;
        private String shippingAddrState;
        private String shippingAddrCountry;
        private String shippingAddrZipcode;
}

Question 3: Is this DTO design apt for that use case? if not, then what would you suggest?

Question 4: The user has multiple addresses. But, Shouldn't the address details be in its own class (something like UserAddressDTO) and then we should add an array/ArrayList of UserAddressDTO in UserDTO??

Question 5: Also please give some insight about how would such kind of DTOs effect the JVM memory. The report would fetch thousands of records.

  • 2
    My first question is why a PersonDTO has a list of PersonDTOs. I don't usually think of "persons" as being one of the properties that a person has. – Tanner Swett Jun 10 '17 at 21:34
  • @TannerSwett Yes! That is the problem I am trying to understand and solve :-) – madan Jun 13 '17 at 9:32
  • This seems a bit backwards to me.... I would have two objects, a UserReportDTO (with the summary and the list containing individual users) and the UserDTO, with the properties that belong to each user. Mixing up those two things in a single, gigantic structure seems really unhealthy to me. – T. Sar Jun 13 '17 at 11:05
5

Question 1: Huge DTO

The goal of a DTO is to transfer data between processes or layers so to reduce the number of method calls.

It is hence not shocking to have a huge list of fields in such a structure. The alternative is to implement the DTO with some more complex structure, such as for example a ResultSet (or a dataset in a non-jaa context), that groups together different entities into one object. The inconvenience is then that the building of such object and the accessing of its elements is more time consuming, that with your flat (and ugly, but efficient) structure.

FYI, here you'll find some benefits and liabilities of the DTO.

Question 2 (updated after your edit)**

Having a list of PersonDTO in the PersonDTO means that the DTO has a recursive structure (e.g. like a tree). So it's not just flat data of one Person, but in fact one Person and a group of related Persons. The data structure is recursive.

This doesn't imply a circular reference, but you are right, some extra care is needed. It all depends on the original data structure:

  1. If the original objects form a hierarchy (for example, a manager and his employees, some of them themselves managers having their own employees), there is no issue to create the DTO. Same if it's another form of acyclic graph.
  2. If the original objects form a graph with possible cycles (for example, a person and her friends), there is a risk of cycling for ever when building the DTO. You can use a cycle detection to prevent transfering several time the same object. However, doing so would mean losing some relations in the transfer. In this case this DTO would be a bad design.

Note also that a same Person could appears several time in the DTO, in different lists (e.g. a part-time employee working for two departments). Unfortunately, during the data transfer, if the objects get deserialized to be transferred over the net and then serialized again, the object's identity (that you have in java with the same reference) might get lost, and you'll end up with two different objects replicating the same data. On the reception side, you'd therefore need to take some extra care as well : you have to decide if you'd detect such identity of objects or if you assume that all persons will always be different.

Conclusion: this structure could be ok, but it requires some information about the original domain objects to confirm.

Alternative: A much safer DTO structure would be to give each person an id, and organise the DTO as a collection of persons with their id. The list of PersonDTO could then be replaced with a list of ids. This is much more flexible, because it would work for all the types of person graphs and without need for any identity detection.

Question 3 and your last edit

If the goal of this DTO structure is only to provide a list of unrelated users, it should have the structure:

class UserDTO {             // Only properties of a single user
    private String userID; 
    private String firstName;
    ...
    private String userType;  
};
class UserListDTO {         // Only properties of the list as a whole
    private int totalDoctorCount; 
    ...
    private List<UserDTO> users;
};

As you see, a proper DTO design is often related to a multiplication of DTO classes. This is a well known topic as you can verify in the drawbacks exposed in this article here.

Inflating a unique DTO to combine the two layers into a single class is a shortcut that does not comply with the principle of single responsibility. Its also ambiguous: should the top user be added to the group or not ? Is he then counted in the statistics ? Shall the users in the list always have an empty list ? Shall they they provide statistics on themselves ? What if one user in the list comes with unexpected users in his list ? All these questions and extra processing, just to avoid one additional class...

In this case, it is a bad design.

  • Just a note: A list of PersonDTO within a PersonDTO doesn't not necessarily causes circular reference, unless the same instance of PersonDTO is allocated in the List. – Laiv Jun 10 '17 at 12:39
  • Thank you @Christophe for the valuable info. Its actually a list of PersonDTO objects. It was a typo. Sorry for that. – madan Jun 10 '17 at 19:04
  • @madan ok. I see your issue, and have edited my answer accordingly. – Christophe Jun 10 '17 at 22:11
  • 1
    @Laiv note that my answer was updated after the question was edited. – Christophe Jun 10 '17 at 22:13
  • 1
    @madan question 4 is about the already mentioned DTO explosion. In principle you'd better go for an addressDTO. Question 5 on performance reminds me that premature optimisation is the root of all evil. But basically, having 5000 strings grouped in one object or spread over 100 object should not fundamentally change the figure – Christophe Jun 13 '17 at 17:16
1

Question 1: Is it a good practice to have such huge DTO classes with so many properties?

As others have mentioned, a DTO can certainly be that large if its consumer needs it. It's difficult to comment on whether such a large DTO is a good practice because DTO serve such a narrow purpose that there is no real variation of "good-practicedness" in using them.

Question 2: Is it a good practice to have a DTO class having an ArrayList of its own type? Wouldn't it cause a circular reference?

In terms of Modelling, a Person isn't composed of multiple other Persons. A Person could have multiple siblings of type Person. The point being a member called persons doesn't convey any relevant information about its relation to the object. On the other hand, it's obvious that a member named siblings describes a familiar relationship between these Person instances.

1

Wikipedia

data transfer object (DTO) is an object that carries data between processes. The motivation for its use is that communication between processes is usually done resorting to remote interfaces (e.g. web services), where each call is an expensive operation. Because the majority of the cost of each call is related to the round-trip time between the client and the server, one way of reducing the number of calls is to use an object (the DTO) that aggregates the data that would have been transferred by the several calls, but that is served by one call only

According to the above definition, if your DTO is addressed to reduce the number of calls between processes, we could argue about the size of the DTO but not its adequacy. On the other hand, if the DTO does not response to any communication strategy, the likely answer is, please, do review your design. Whoever is going to consume the DTO will have a hard time exploring, analysing and extracting data from such monstrosity.

Regarding the second question, there could have circular reference if by some reason the same DTO is referenced somewhere in the List or referenced by any of the DTO's inner list. And so on.


Updat after your edit

@Christophe already has told what we all think (more or less). if UserDTO carries with all the data required for the report, you provided it with too many responsabilities.

Consider the next 2 options:

  • modelling the web resource ReportDTO composed by others DTO and its own local attributes.

  • doing more calls to the server. We know that the aim of the DTO is the opposite to this, but don't we go mad, reducing 50 calls to 1 is excellent, but don't do It at risk of implementing the One DTO (one DTO to bring them all, and into darkness bind them). Cutting-down the number of calls from 50 to 5 it's still a big deal improvement.

  • Thank you @Laiv for the valuable info. Its actually a list of PersonDTO objects. It was a typo. Sorry for that. Yes the Data needs to be serialized. The list too. I need a JSON response to be sent via REST – madan Jun 10 '17 at 19:06
0

Fifty properties seems a little excessive. You should try and keep them as trim as possible. Only include what you need. Though I've seen some which are 10-15 properties.

And in regards to the ArrayList, you can have one without an issue. The circular reference would only occur if for some reason you stored the actual item that's doing the work as a reference or something weird like that.

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.