C#•3mo ago

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

Angius•3mo ago

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

Billy•3mo ago

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

Angius•3mo ago

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

Billy•3mo ago

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"?

Angius•3mo ago

With a .Select() for example

Billy•3mo ago

But 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; }

Angius•3mo ago

var patient = await _context.Patients
    .Where(p => p.Id == id)
    .Select(p => new PatientDto {
        FirstName = p.FirstName,
        LastName = p.LastName,
        // ...
    })
    .FirstOrDefaultAsync();

var patient = await _context.Patients
    .Where(p => p.Id == id)
    .Select(p => new PatientDto {
        FirstName = p.FirstName,
        LastName = p.LastName,
        // ...
    })
    .FirstOrDefaultAsync();

Angius•3mo ago

Angius•3mo ago

You don't Unless you just changed that

Billy•3mo ago

Ah, I see, my bad. I thought FindAsync was like to a linq query

Angius•3mo ago

No, it resolves the query. This is the moment the query gets executed on the database

Billy•3mo ago

Understood, thanks for that, will update

Angius•3mo ago

Also, it's generally a better idea to use a y.FromX() pattern than x.ToY()

Billy•3mo ago

But is it right to return the Dto from the Service layer? Or do I need yet another Dto between Service and Controller

Angius•3mo ago

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 lol

Billy•3mo ago

Dare 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)?

Angius•3mo ago

If you were to follow the clean architecture thingamajig philosophy, then yes You'd have a mapping at every stage, basically

Billy•3mo ago

Ok, and in a service layer, I "always" map to the Model before doing any work, right?

Angius•3mo ago

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

Billy•3mo ago

This mapping, can you explain this a bit more?

Angius•3mo ago

Sure This is, generally, ill-advised.

public class Source
{
    public required int Id { get; set; }
    public required string Name { get; set; }
    public Target ToTarget() => new Target { Name = this.Name };
}
public class Target
{
    public required string Name { get; set; }
}

public class Source
{
    public required int Id { get; set; }
    public required string Name { get; set; }
    public Target ToTarget() => new Target { Name = this.Name };
}
public class Target
{
    public required string Name { get; set; }
}

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:

public class Source
{
    public required int Id { get; set; }
    public required string Name { get; set; }
}
public class Target
{
    public required string Name { get; set; }
    public static Target FromSource(Source s) => new Target { Name = s.Name };
}

public class Source
{
    public required int Id { get; set; }
    public required string Name { get; set; }
}
public class Target
{
    public required string Name { get; set; }
    public static Target FromSource(Source s) => new Target { Name = s.Name };
}

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 either

Billy•3mo ago

I 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

Angius•3mo ago

Lemme fire up Paint right quick

Billy•3mo ago

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)

Angius•3mo ago

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...

Angius•3mo ago

With a FromX pattern, the db access project doesn't need to know those classes, doesn't need to reference those projects

Angius•3mo ago

No cycle

Billy•3mo ago

Ah I see. Then make a Dto project and a Models project and you eliminate this problem though, right?

Angius•3mo ago

Yes, but then you have a bunch more projects for no reason other than solving an issue you yourself introduced Again, I favour simplicity

Billy•3mo ago

Yeah, makes sense Thank you ^ the last question that is throwing me off a little

Angius•3mo ago

I'd use a different DTO for incoming data and for outgoing data

Billy•3mo ago

Ok, makes sense - but here we're adding complexity 😄

Angius•3mo ago

This is the complexity that's worth it lol

Billy•3mo ago

But I prefer it ha Thanks for the tips - good chat 😄

Angius•3mo ago

Anytime

Billy•3mo ago

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...

Angius•3mo ago

Go through patients

Billy•3mo ago

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

Angius•3mo ago

public async Task<List<PatientDto>> GetDoctorPatientsById(int id)
        {
            return await _context.Patients
                .Where(p => p.DoctorId == doctor.Id)
                .Select(p => p.ToDto())
                .ToListAsync();
        }

public async Task<List<PatientDto>> GetDoctorPatientsById(int id)
        {
            return await _context.Patients
                .Where(p => p.DoctorId == doctor.Id)
                .Select(p => p.ToDto())
                .ToListAsync();
        }

Billy•3mo ago

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

Angius•3mo ago

you can query doctor with patients and return only patients

public async Task<List<PatientDto>> GetDoctorPatientsById(int id)
        {
            bar doc = await _context.Doctors
                .Where(d => d.Id == id)
                .Select(d => new {
                    Patients = d.Patients.Select(p => p.ToDto()) 
                })
                .FirstOrDefaultAsync();

              return doc is null ? null : doc.Patients;
        }

public async Task<List<PatientDto>> GetDoctorPatientsById(int id)
        {
            bar doc = await _context.Doctors
                .Where(d => d.Id == id)
                .Select(d => new {
                    Patients = d.Patients.Select(p => p.ToDto()) 
                })
                .FirstOrDefaultAsync();

              return doc is null ? null : doc.Patients;
        }

Billy•3mo ago

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

Angius•3mo ago

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 possibilities

Billy•3mo ago

Yep, 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

Billy•3mo ago

Forgot to push: https://github.com/billymaat/MedTrackDash

GitHub

GitHub - billymaat/MedTrackDash

Contribute to billymaat/MedTrackDash development by creating an account on GitHub.

Gaming

Programming

Model, Dto, Entity - Id?