Naming conventions with Dto
I've got a PatientDto and it has an Id in it from entity core. Sometimes I want a PatientDto without an Id, i.e. if I've got an endpoint GetPatientById(int id) there isn't much point in returning the Id. Should I just returning it anyway and avoid the hassle or should i have a different dto?
35 Replies
I guess you are mixing the DataTransferObject wit the the main database Entity. Your main classes that serves as a row in database are your entites and the classes/records you use for data exchange such as returning respones or recieving some data are DTOs. So yes you need a DTO if you want to customize your response or input data.
In this case, I'd say keep the ID. But in other cases, where it matters more, you do want more specific DTOs and having many is not only usual, but expected and intended.
I usually don't use the
Dto
suffix at all - I call my dtos by what made them. RegisterPatientResponse
is the DTO for Patient
when it was just createdI try to make my DTO names more descriptive
Yeah and normally you don't hide ids in such cases
PatientPostDto
, PatientGetDto
, PatientSimpleDto
, etcwhat properties to hide/show should be based on the intended use. Id is pretty safe to include almost everywhere, as its not uncommon for a frontend to store the object they fetched for later use, and them having to stitch in the ID manually is extra work
Eh, if I don't need the ID I don't include it
It's not needed to, dunno, display a blogpost
for a
GetXById
I would not remove the IDyou might need the ID in serveral cases later on like if you wanna play with that specific object
Then I add the property to the DTO ¯\_(ツ)_/¯
A matter of choice then 🙂
preference*
Having the ID in your DTO's is always a good idea. I usually only have 1 DTO for each of my entities. I make all the properties nullable and tell the controllers to ignore null properties. So any property that have a null value, won't be included.
You are an evil evil person.
It makes life simplier.
Also a very ironic name you have there, as doing this will make your swagger contract be a mess
swagger looks at the types returned by your api methods, and if you use the same DTO everywhere, it appears as if they all return COULD return everything
so yeah, if you wanna do that go ahead... but please don't recommend that approach to anyone
I'm the only developer. So it makes since to me.
Again, thats fine. But you can't assume everyone else is also a solo dev, or won't care about API contracts
That's true. I haven't really thought of that.
Another question:
I've got an AppointmentDto that contains Date and DoctorId and PatientId. Let's say I have an GetAppointOnDate(DateTime date) and it would return List<AppointmentDto>. How would you handle this on your client side? I imagine that the DoctorId would appear many times in different AppointmentDto. Would I keep calling the GetDoctor(int id) end point? Or keep a cache or something on the client side for each call. The cache is added complexity is my main concern...
First thing, I'd change your
GetAppointmentsOnDate
to accept a DateOnly
🙂
if it gets all the appointsments on that date, it shouldnt take a time componentOne solution would be to tell the JsonSerializer to
IgnoreCycles
.And yeah, either you have the AppointmentDto contain a DoctorDto inside of it, which leads to a lot of duplicated data, or you only return the DoctorId and let the FE fetch it.
we do the second approach at work for endpoints we know often return a lot of repeat data, and we use a "cache" (its just the frontend data store)
I'm curious, ignoring cycles, is it a bad idea?
not sure how IgnoreCycles solves anything here, as the problem isn't
Appointment -> Doctor -> Appointment
yes, you should model your DTOs so there are no cycles anyways
I think it mostly comes down to ease of use and how many appointments you are likely to get on a single date
if its 1000+, then the repeat data might become a problem
if its less then that, its not really, and the ease of use of simply including the doctors info with the response wins out in my book
alternatively, add paginationI have actually had this issue before. By telling it to ignore cycles it only keeps 1 object. Instead of having for instance
Doctor -> [Appointment(Doctor -> [Appointment])]
yeah, but as said, if you dont reuse dtos and have too many props, the
DoctorDto
in this case wouldn't have any back-nav props
it would just be a simple data object representing the doctor, it wouldnt also include a list of appointmentsThanks!
How are you mapping Entity to DTO btw?
I've got an extension method PatientDto ToDto(this PatientEntity) etc, or do you mean what's in the ToDto method? I'm still working on the Appointment side of things
Make sure it translates well to SQL, and EF isn't doing something weird like fetching everything and mapping client-side
^
You might want to have a look at something like
Mapperly
which will allow you to have IQueryable<Entity>
to IQueryable<Dto>
mapping. All source generated too, so no weird runtime performance lossGood to know, thank you. I'll keep an eye out for it
My ToDto:
/// <summary>
/// Maps an AppointmentEntity to an AppointmentDto.
/// </summary>
/// <param name="appointmentEntity">The AppointmentEntity to map.</param>
/// <returns>The mapped AppointmentDto.</returns>
public static AppointmentDto ToDto(this AppointmentEntity appointmentEntity)
{
return new AppointmentDto
{
Id = appointmentEntity.Id,
DateTime = appointmentEntity.DateTime,
PatientId = appointmentEntity.PatientId,
DoctorId = appointmentEntity.DoctorId
};
}
My call in EF core:
var appointments = await _context.Appointments
.Where(a => DateOnly.FromDateTime(a.DateTime) == date)
.Select(a => a.ToDto())
.ToListAsync();
My AppointmentEntity does have PatientEntity and DoctorEntity properties, but I don't map them. But I guess EF is getting them anyway yeah? That's what you mean?Yeah, from what I recall EF doesn't really like anything with a
return
, it likes expressionsyup
Try