CODEWITHSUNDEEP
c#asp.netasp.net-mvcdto
Bart
Bart

Reputation: 53

ASP.NET Multiple DTOs to one Model and validation inside DTO

In my web app I have class with many properties. User is able to modify these properties with bunch of selectboxes, each responsible for one property. Everytime when given property is changed, the change event is triggered and ajax sends the new value of this property to Update method in the Controller.

So far I have one UpdateDto which consists of nullable fields. In my Update controller I check each DTO's field if it is null or not. If it's not null it means that user want to change this property and it is updated and saved in database.

Unfortunately the Update method's code looks a little bit ugly for me. It checks each property and it's quite long. Have a look:

Update Method in the controller:

public IHttpActionResult UpdateScrumTask(int id, ScrumTaskDetailsDto scrumTaskDto)
{
var scrumTaskFromDb = _context.ScrumTasks
    .SingleOrDefault(s => s.Id == id);

if (scrumTaskFromDb == null)
    return NotFound();

if (!ModelState.IsValid)
    return BadRequest();

if (!string.IsNullOrWhiteSpace(scrumTaskDto.UserId))
{
    var user = _context.Users
        .SingleOrDefault(u => u.Id.Equals(scrumTaskDto.UserId));

    scrumTaskFromDb.UserId = user?.Id;
}
else if (scrumTaskDto.EstimationId != null)
{
    var estimation = _context.Estimations
        .SingleOrDefault(u => u.Id == scrumTaskDto.EstimationId.Value);

    scrumTaskFromDb.EstimationId = estimation?.Id;
}
else if (scrumTaskDto.Priority != null)
{
    if (scrumTaskDto.Priority.Value == 0)
        scrumTaskDto.Priority = null;

    scrumTaskFromDb.Priority = scrumTaskDto.Priority;
}
else if (scrumTaskDto.TaskType != null)
{
    scrumTaskFromDb.TaskType = scrumTaskDto.TaskType.Value;
}

_context.SaveChanges();

return Ok();
}

UpdateDTO:

public class ScrumTaskDetailsDto
{    
    public int? EstimationId { get; set; }

    [Range(0, 5)]
    public byte? Priority { get; set; }

    [Range(0, 2)]
    public TaskType? TaskType { get; set; }

    public string UserId { get; set; }
}

Note that these properties are also nullable in the database. That's why if for example UserId is not found the property in database is set to null.

I wonder how it should look like. What is better solution?

Another question is: Should I use validtion (DataAnnotation) inside DTO? If not, what is alternative solution?

Upvotes: 1

Views: 983

Answers (1)

Arman Hamid Mosalla
Arman Hamid Mosalla

Reputation: 3355

I'd suggest couple of improvements:

  1. Separate your query logic from your core business logic, using repository pattern or query and command object using a library like MediatR, also controller only should call other methods to do something for it and return a response, it shouldn't have any query or any other logic
  2. The DTO looks ok to me, as long as it's centered around one specific task
  3. I would definitely separate the update from controller, as for it being one method or more, it's 35... lines, which is not ideal, maybe you could separate it into two methods, one responsible for validation and one responsible for the actual update, also you can use dependency injection to decouple the update and validation method from your controller

Upvotes: 2

Related Questions