6

I found an article by Martin Fowler that has made me doubt my design choices, particularly about data objects. Martin Fowler has written an article about what he thinks is a code smell.

In this article, he says:

The best smells are something that's easy to spot and most of time lead you to really interesting problems. Data classes (classes with all data and no behavior) are good examples of this. You look at them and ask yourself what behavior should be in this class. Then you start refactoring to move that behavior in there. Often simple questions and initial refactorings can be the vital step in turning anemic objects into something that really has class.

Now, on to my own use case of data classes/objects/DTOs/whatever. :-)

I am writing a set of classes that move credits from our company's sim to the sim of a recipient. This process has three stages:

  1. Validation: at this stage, the data is validated – have we moved credits to this person's account before? Have we handled this item before? Can we retry, or have we reached the "manual mode" threshold?
  2. Process: at this stage, we do the actual work of moving the credits from one account to the other. We use the objects retrieved in the validation phase, to lower the number of queries to the database, and also to improve our application's performance.
  3. Post-process: at this point, we check the data and if the data must be auto-corrected (I.E. the user has entered wrong information and we know how to automatically fix it), we mark it for autocorrection.

Now, the items 2 and 3 are using a lot of data that is retrieved or initialized in stage 1: the event manager, the order and payment and transaction objects, and so on. What we do is the following:

  1. The validation object stores all of its retrieved data that is useful to the rest of the process in a data transfer object (DTO). This object has only setters and getters.
  2. The rest of the classes (the process class and the post-process class) accept the DTO as their constructor argument and do their processing, event management, and data manipulation on those objects. In the end, those objects are persisted in the database.

Is the data object (or the data transfer object) not recommended here? Should we use another way for sharing data between these three classes?

Improve this question
7
  • possible duplicate of What is the point of using DTO (Data Transfer Objects)?
    – gnat
    Nov 17, 2014 at 12:00
  • Thanks a lot for editing rather than downvoting/voting to close. What I don't understand is why DTOs are code smell. I am using our project as an example – I can't think of this DTO having some behavior, or to be removed entirely.
    – Parham Doustdar
    Nov 17, 2014 at 12:03
  • 1
    The point of OOP is to encapsulate state and behaviour. If an object has some named, typed attributes, and there is code that works with these attributes, then as far as possible, that code should be within that class, and not in some external, easily-overlooked static "service" or "manager" class.
    – Kilian Foth
    Nov 17, 2014 at 12:05
  • Well, none of the classes I mentioned are static, or managers/services/handlers. However, if I put all the code for the whole process (validation, process, and post-process) into one class, the class would be too bulky. True, it would still only handle the process of moving credits to someone (I.E. it won't violate SRP), but having a lot of validation code in this class leaves a bad taste in my mouth. As soon as you extract the validation, however, you get the problem of having to query the database once for validation, and once for the actual process.
    – Parham Doustdar
    Nov 17, 2014 at 12:12
  • 1
    Code smells are just a heuristic useful for finding places that have the potential for improvement. They usually aren't hard-and-fast rules. Finding the smells helps you map out your code; the "smelliest" places can draw more attention. Some smells you may decide aren't that odorous after all. :)
    – Allan
    Nov 17, 2014 at 16:11

1 Answer 1

Reset to default
6

Should we use another way for sharing data between these three classes?

No, that is not the point of what Fowler is talking about. The usage of a DTO is probably fine (Fowler himself recommends it, especially in a remote scenario). The point is that during refactoring, such a DTO might become a better place for operations which work exclusively on the data stored within the DTO (that actually does not mean to put the whole validation, processing and post-processing code in there).

Candidates are typically operations which might be reused in more than one of the steps you described above. Another indicator is when you start implementing something like a FooDTOManager class for a specific class FooDTO, instead of implementing the operations directly in FooDTO. What you should consider is the name your give to the DTO - it will ideally express some kind of abstraction and some kind of responsibility. Use this responsibility as a guideline when deciding about which operations you put into the DTO, and which not.

However, if the only responsibility of your DTO objects (currently) is to transfer some data from step x to step y, then leave it as it is. Later, when your program evolves, you might find operations which may be suited as operations for this DTO - if there are currently none, it is ok, don't start to seek for a different solution.

Improve this answer
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.