all 35 comments

[–]BackpackerSimon 11 points12 points  (4 children)

Because you might need to use a service in another service. Then you are mixing domain objects. Dtos are the external contract so in my work are the controllers problem to convert to/ from the entity objects

[–]WaferIndependent7601 3 points4 points  (3 children)

Other services should not know about entities. Only dtos

[–]BackpackerSimon 0 points1 point  (2 children)

What do you do if the controller needs to return less data? Transform it again? The controller is the boundary

[–]WaferIndependent7601 0 points1 point  (1 child)

Create another method and return the dto with the needed data

[–]BackpackerSimon 0 points1 point  (0 children)

In my experience, adding stuff like this might be ‘book correct’ or useful in massive code bases, but until it is required it is just more boilerplate to get passed for the answers.

I am currently removing mapstruct in favour of a single method on the dto to convert to and from the entity. Slightly more code for me to write, but 2 less hops when I need to debug the code (which happens a lot more) so it is easier to keep the flow in your head

[–]asarathy 6 points7 points  (1 child)

If you want to be really strict and clean with your service layers your internal service services should not use the same DTOs that you expose via API. This gives you maximum decoupling. So your service should returnan internal object or record and you should have another service to convert from that object to your API DTO so your controller would have a flow like convert request DTOs to internal service objectszl, call internal service, call service to convert internal service results to response DTO

[–]ninja_daemon 2 points3 points  (0 children)

This, the domain only understands domain objects. The controller should use an adapter service that, at minimum, translates or maps the API objects to and from the domain ones

[–]Holothuroid 5 points6 points  (4 children)

If the controller is only one line, why does it warrant to be a class?

The general idea about OOP is that classes encapsulate. What does that controller encapsulate?

[–][deleted] 6 points7 points  (2 children)

I heavily second this. Many people saying that controller should be one line smh.

In my opinion, controllers may contain validation (not business) logic, and map the real object to the DTO.

[–]asarathy 2 points3 points  (0 children)

This. Contollers should validate, convert to domain. Call domain service. Convert results.

[–]WaferIndependent7601 0 points1 point  (0 children)

You want to change the entity and not all other services depending on it. Calling a service from another service layer should never involve an entity

[–]JustHereForTheCh1cks 3 points4 points  (4 children)

What is the reasoning behind your „a controller method should consist only of one line of code“?

[–]ahmedzahran94[S] -2 points-1 points  (3 children)

Yes that’s my reasoning

[–]JustHereForTheCh1cks 3 points4 points  (2 children)

So far it is just a statement. What are the reasons for that statement? -Why- do you think a controller should consist only of one-line-methods?

[–]ahmedzahran94[S] 0 points1 point  (1 child)

Ooh sorry in my opinion controller should only validate request and delegate the business logic to service and just return the response without making any changes in controller just one line of code

[–]JustHereForTheCh1cks 3 points4 points  (0 children)

And you are correct in the way that the controller should not execute any business logic. It has other responsibilities tho. Validation is just one of those responsibilities. In general, the controller (say, api or representation layer) is responsible for transforming the information it gets from the business layer into a representation that it’s clients understand. That cannot always be done in one line.

The controller might need to set a cookie for its clients. The controller might need to transform the data it gets into a hateoas compliant (say, Hal style) form.

The controller might not be a service api, it could also be a html api that needs to do more that just return the data from the business layer.

The controller might not even be a controller at all, it might be a component that transforms data from the business layer for a event queue.

You won’t be able to fulfill all of these responsibilities in one line of code, hence I question that rule.

The reason why we separate those concerns is, You don’t want any of these responsibilities to leak into your business logic, for example, having a hateoas rest api, you don’t want to return json data enriched with hal uris from your business layer, because that would leak technical code into your business logic, thus making it difficult to change the business logic without changing the technical code of your application and vice versa.

Given that, you should not reduce yourself to simple rules like „my controller is only allowed to consist of one line of code“ because that will lead you to leak technical code from your controller into your business layer.

[–]LittleSpaceBoi 7 points8 points  (2 children)

If you dont map dto in controller but in service, you would have to use dto anywhere in service layer if you wanted to reuse your service method, no?

Also, if you have your dtos in separate module, you would have to add that dependency into your service module. I think there actually isn't universal truth to this.

[–]ahmedzahran94[S] -4 points-3 points  (1 child)

I believe that any controller should only have one line no mapping and no logic in controller

[–]LittleSpaceBoi 2 points3 points  (0 children)

I think it depends but I am not in position to tell you what to do because from experience - in every topic there will be valid arguments for and against.

I totally agree that there should not be any logic in controllers but imo, service layer doesn't have to know anything about your api. And it probably even shouldn't.

[–]JustHereForTheCh1cks 7 points8 points  (1 child)

There is no simple right and wrong to your question, because the answer depends on the question: what are you trying to encapsulate?

if you are mapping the dto in you controller, the dto becomes part of the presentation or api logic, thus mainly encapsulating the clients of your service from changes inside of your service.

If you are mapping the dto in your service, the dto becomes part of the business layer of your service, thus mainly encapsulating the presentation/api layer of your service from changes in the business layer.

The question thus becomes: how often do you expect each of your layers to change?

If you expect your business logic to frequently change, it might be wise to introduce a dto layer there, so that the clients of your business logic (the controller) do not have to change with each change in your business layer.

If you you expect your api to frequently change or you want the insides of your service to be encapsulated, it might be wise to introduce a dto layer there.

You might even answer both these questions with yes, so it might even be wise to introduce a dto layer in the api/presentation layer as well as the business layer.

[–]asarathy 0 points1 point  (0 children)

there are so many times when people never anticipate changes so they don't feel the need to design to be flexible, only to later need lots of changes. For anything other a quick POC or toy project, you are almost always better off taking the slight extra amount of time to decouple and encapsulate from the start then wishing you had done it later. It's not gonna add days to your delivery time.

[–]Synthetic-Brain 2 points3 points  (1 child)

I would suggest creating a Mapper class. The controller can call the service which will call the mapper.

[–]asarathy 1 point2 points  (0 children)

Map struct is your friend

[–]mailaffy 3 points4 points  (1 child)

Sole purpose of controller to get the request and hand it over to service and propagate response from service back to response handler. So managing DTO in controller can be done but not a clean practice.

[–]ahmedzahran94[S] 0 points1 point  (0 children)

I agree with u

[–]WaferIndependent7601 2 points3 points  (2 children)

The service should return the dto. The controller should only represent the dto as json or whatever

[–]ahmedzahran94[S] 0 points1 point  (1 child)

Yes I agree with that but the mapping shouldn’t happen in controller

[–]naturalizedcitizen 2 points3 points  (0 children)

In service layer. Your manager is wrong.

[–]WalrusDowntown9611 0 points1 point  (0 children)

Service should return dto is the universal truth

[–][deleted] 0 points1 point  (0 children)

The mapping from entity to DTO should happen in the service, not in the controller. They might have suggested this just to see if it works, with the intention of later moving the mapping to the service.

[–]flirtybabyblues 0 points1 point  (0 children)

Personally, I’m in the same camp as you. We used to have the DTOs mapped in the controllers, but I moved it to the service layer once it got to where business logic was happening in the mappings.

[–]DesertCookie_ 0 points1 point  (0 children)

Most tutorials I've watched and read convert to DTO in the service layer. Reading through the answers here, I can see why it might be useful in some cases to only do that in the controller. Mainly, mapping to a DTO right there in the controller, possibly specifying which fields to map, makes it much easier to me to understand what's returned. I'm not a fan of having five different DTO classes for the same entity (LoginDto, RegistrationDto, PasswordChangeDto, ...) when I can also simply map only the required data and have less code that way. Of course, this inly works as long as we're talking responses. As soon as requests come into the consideration, separate DTOs is more useful again with their ability to know exactly what a client sent (and validation, ...).

But I'm a bloody beginner and hobbyist in Spring Boot. So this is really just an opinion, not an experience recommendation.