Which approach is right while creating a service for your update method?

Question

I was following a tutorial on Web API and I saw the creator creating his service method as nullable ( Task<Comment?> Update(CommentUpdateDto comment) ) and he later used it like following which made sense:

[HttpPut("{id:int}")]
public async Task<IActionResult> Update([FromRoute] int id, [FromBody] CommentUpdateDto commentUpdateDto)
{
    if (!ModelState.IsValid)
        return BadRequest(ModelState);

    var comment = await _commentRepo.UpdateAsync(id, commentUpdateDto.ToCommentFromUpdate());

    if (comment == null)
    {
        return NotFound("Comment not found!");
    }

    return Ok(comment);
}

Now I want to use same approach in my ASP.NET Core MVC app, but for some reason it doesn't make sense. It is probably because I am using fluent validation with auto mapper.

My CategoryService:

public async Task<Category?> UpdateCategoryAsync(UpdateCategoryDto updateCategoryDto)
{
    var category = await GetCategoryByIdAsync(updateCategoryDto.Id);

    if (category == null)
        return null;

    mapper.Map(updateCategoryDto, category);

    await unitOfWork.GetRepository<Category>().UpdateAsync(category);
    await unitOfWork.SaveChangesAsync();

    return category;
}

My controller action:

[HttpPost]
public async Task<IActionResult> Update(UpdateCategoryDto updateCategoryDto)
{
    var category = mapper.Map<Category>(updateCategoryDto);
    var result = validator.Validate(category);
    var exists = await categoryService.Exists(category);

    if (exists)
        result.Errors.Add(new ValidationFailure("Name", "This category name already exists"));

    if (result.IsValid)
    {
        await categoryService.UpdateCategoryAsync(updateCategoryDto);
        return RedirectToAction("Index", "Category", new { Area = "Admin" });
    }

    result.AddToModelState(ModelState);

    return View(updateCategoryDto);
}

I tried to modify it like this:

public async Task<IActionResult> Update(UpdateCategoryDto updateCategoryDto)
{
    var category = mapper.Map<Category>(updateCategoryDto);
    var result = validator.Validate(category);
    var exists = await categoryService.Exists(category);

    if (exists)
        result.Errors.Add(new ValidationFailure("Name", "This category name already exists"));

    if (result.IsValid)
    {
        var value = await categoryService.UpdateCategoryAsync(updateCategoryDto);

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

        return RedirectToAction("Index", "Category", new { Area = "Admin" });
    }

    result.AddToModelState(ModelState);

    return View(updateCategoryDto);
}

The thing is if my updateCategoryDto is null, I cannot even pass the validation as my category will be null, so my modification doesn't change anything. I want to know what changes I should make in order to have a logical flow. Should I just use my service method as Task<Category> instead of Task<Category?> or do I have to make changes in my controller action.

Note that I am a self-taught beginner, so any suggestions or advice is valuable for me. If you think I can make changes in my code for better, please share it with me. Thanks in advance!

Answer 1

The thing is if my updateCategoryDto is null

If the input (updateCategoryDto) is null then there's something wrong with the request. In that case, return 400 Bad Request:

if (updateCategoryDto is null)
    return new BadRequestResult();

Alternatively, you may wish to use one of the alternative types, like BadRequestObjectResult, if you want to supply additional information to the caller.

Often, however, the ASP.NET framework is going to intercept the request even before that, so that in practice, your action method never gets called with a null value.