Model, Dto, Entity - Id?
Hi,
I'm messing with a simple CRUD application.
My PatientEntity looks like this:
public class PatientEntity
{
public int Id { get; set; }
public string FirstName { get; set; }
public string Surname { get; set; }
public int Age { get; set; }
public string Gender { get; set; }
}
And my model looks like this (no id):
public class Patient
{
public string FirstName { get; set; }
public string Surname { get; set; }
public int Age { get; set; }
public string Gender { get; set; }
}
And my dto looks like this (no id):
public class PatientDto
{
public string FirstName { get; set; }
public string Surname { get; set; }
public int Age { get; set; }
public string Gender { get; set; }
}
And currently a method in my DatabaseService looks like this:
public async Task<Patient> GetPatientById(int id)
{
PatientEntity patientEntity = await _context.Patients.FindAsync(id);
if (patientEntity != null)
{
return patientEntity.ToPatient(); // i.e. returning the Patient model
}
_logger.LogInformation("Patient not found in the database.");
return null;
}
Should my DatabaseService return a PatientEntity i.e. with an id, and my PatientDto contain the Id? My PatientDto is returned by a web api controller.
I feel like my PatientDto should contain the Id. I guess the problem is that the DatabaseService is returning a Patient (Model) rather than the PatientEntity. Does it make sense to return the model from the Database service?
Here's the git:
https://github.com/billymaat/MedTrackDash
GitHub
GitHub - billymaat/MedTrackDash
Contribute to billymaat/MedTrackDash development by creating an account on GitHub.
46 Replies
Wym "database service"?
You have one single service that does everything?
Also, by getting an entity from the database first, and then mapping it, you lose a good chunk of the benefit of the mapping. Map as a part of the query
Well I thought I should have a layer between the AppContext (EF) and my controller, hence he DatabaseService. At the moment, responsible for getting anything/adding to the db
Sure, having a service layer there is prudent
But not a god-service that does everything
One service for patients, one for rooms, one for staff, one for medicine, and so on
Yes, that makes sense. At the moment I only have Patients, but point taken 🙂
What do you mean by "map as a part of the query"?
With a
.Select() for exampleBut I do that, don't I?
public async Task<List<Patient>> GetAllPatients()
{
var patients = await _context.Patients
.Select(p => p.ToPatient())
.ToListAsync();
_logger.LogInformation("Retrieved all patients from the database.");
return patients;
}
You don't
Unless you just changed that
Ah, I see, my bad. I thought FindAsync was like to a linq query
No, it resolves the query. This is the moment the query gets executed on the database
Understood, thanks for that, will update
Also, it's generally a better idea to use a
y.FromX() pattern than x.ToY()But is it right to return the Dto from the Service layer? Or do I need yet another Dto between Service and Controller
The source should not know of the target
I'd say it's perfectly fine to return a DTO
But I'm a fan of not introducing much complexity in the first place. Pretty much all CQRS handlers in my current app just return a
HttpResult object straight lolDare I say, is the "proper" way to have a PatientApiDto, that is returned by the controller for what ever consumes it, and a PatientDto, that is used to communicate between the web layer and the service layer (and maybe between various services, if not using the Patient model directly)?
If you were to follow the clean architecture thingamajig philosophy, then yes
You'd have a mapping at every stage, basically
Ok, and in a service layer, I "always" map to the Model before doing any work, right?
I believe so, yes
In general, as a rule of thumb, you should not be passing database entities around
Any other mapping you want to make... up to you
This mapping, can you explain this a bit more?
Sure
This is, generally, ill-advised.
If you have a project that has all your data access code, and two separate projects that rely on this one... well, the shared project would have to reference those two dependent projects. Which would create a dependency cycle
Bad
This:
does not introduce this issue
You can have a separate API project with a
Target class, and a separate Windows service project with a TargetSimple class, for example
The shared project would not need to know of eitherI think I understand what you mean. But these are extension classes / methods that do the mapping rather than living in the classes themselves
When I say think , I think I would need to end up in this dependency problem
Lemme fire up Paint right quick
Thanks 🙂 And while you do that, the other concern I have:
Should my DatabaseService take PatientDto as as a parameter, i.e. Add(PatientDto patientDto)? But now my patientDto has an Id property, which is not relevant in this context. Should I just make sure my patientDto.Id = 0 before trying to add anything? (not enough EF experience yet to know what happens if Id is non zero and exists already)
So, when you have a
ToX pattern, the database access project needs to know of the target classes, so it needs to reference the projects that contain those target classes.
But those projects need to reference the database access project to, well, access the database.
We get a dependency cycle, where A depends on B depends on A depends on B depends on...
With a
FromX pattern, the db access project doesn't need to know those classes, doesn't need to reference those projects
No cycle
Ah I see. Then make a Dto project and a Models project and you eliminate this problem though, right?
Yes, but then you have a bunch more projects for no reason other than solving an issue you yourself introduced
Again, I favour simplicity
Yeah, makes sense
Thank you
^ the last question that is throwing me off a little
I'd use a different DTO for incoming data and for outgoing data
Ok, makes sense - but here we're adding complexity 😄
This is the complexity that's worth it lol
But I prefer it ha
Thanks for the tips - good chat 😄
Anytime
More questions about the IQueryable 🙂 I've got a Doctor that has Patients and I want to get the Patients from the database.
public async Task<List<PatientDto>> GetDoctorPatientsById(int id)
{
var doctorEntity = await _context.Doctors
.FirstOrDefaultAsync(d => d.Id == id);
if (doctorEntity == null)
{
return null;
}
return doctorEntity.Patients
.Select(p => p.ToDto())
.ToList();
}
I get the feeling this is the wrong way to do it
public async Task<List<PatientDto>> GetDoctorPatientsById(int id)
{
var patients = _context.Doctors
.Where(d => d.Id == id)
.Take(1)
.SelectMany(d => d.Patients);
return await patients
.Select(p => p.ToDto())
.ToListAsync();
}
This doesn't seem the prettiest either. And I'd like to have returned null if the doctor id wasn't found
Next iteration:
public async Task<List<PatientDto>> GetDoctorPatientsById(int id)
{
var doctor = _context.Doctors
.Where(d => d.Id == id)
.Take(1);
if (await doctor.FirstOrDefaultAsync() == default)
{
return null;
}
var patients = doctor.SelectMany(d => d.Patients);
return await patients
.Select(p => p.ToDto())
.ToListAsync();
}
Still don't like the SelectMany...
Go through patients
public async Task<List<PatientDto>> GetDoctorPatientsById(int id)
{
var doctor = await _context.Doctors
.Where(d => d.Id == id)
.FirstOrDefaultAsync()
if (doctor == null)
{
return null;
}
return await _context.Patients
.Where(p => p.DoctorId == doctor.Id)
.Select(p => p.ToDto())
.ToListAsync();
}
Is that how you would handle it? You have to do the first query for the doctor though
ah
haha, thank you
But still, no way to identify whether it is an invalid doctor id, right?
Unless I do the query on the Doctors first for doctor id
you can query doctor with patients and return only patients
Thanks
That makes sense
I guess I can't use ToListAsync() here, but it doesn't matter because or the FirstOrDefaultAsync()? Or am I losing out with something here
Yeah
With this method, three possible states are... possible
1.
doc is null, so there was no doctor with this ID
2. doc is not null but the list of Patients is empty
3. doc is not null and does have patients
As opposed to using the .ToListAsync() method that would result in an empty list in both of the first two possibilitiesYep, makes sense
More questions on this. I've got a PrescriptionDto now and I return it from my API. I have a PrescriptionEntity and MedicineEntity. My API returns the MedicineId rather than the Medicine itself. How would you usually handle this? Is it better to return the MedicineDto in the PrescriptionDto rather than the MedicineId? I know a frontend could handle this, just wondering what is typically done? I've updating my git
Forgot to push: https://github.com/billymaat/MedTrackDash
GitHub
GitHub - billymaat/MedTrackDash
Contribute to billymaat/MedTrackDash development by creating an account on GitHub.