Method with many if and Return

Question

I think my code is too polluted with a lot of if.

[ValidateModelAttribute]
[HttpPut("{id}")]
public IActionResult Put(int id, [FromBody]UserDto model)
{
    try
    {
        if (model == null || model.Id != id)
            return BadRequest();
        var originalUser = context.User.AsNoTracking().FirstOrDefault(x => x.Id == id);
        if (originalUser == null)
            return NotFound();
        var updatedUser = model.MapTo<User>();
        var userBd = context.User.FirstOrDefault(x => x.Login == updatedUser.Login && x.Id != updatedUser.Id);
        if (userBd != null)
            return StatusCode((int)HttpStatusCode.Conflict, userBd);
        context.Entry(originalUser).Context.Update(updatedUser);
        context.SaveChanges();
        return Ok(updatedUser);
    }
    catch (Exception ex)
    {
        return BadRequest(ex.Message);
    }
}

Is there any more elegant way?

Answer 1

• The name Put does not reveal what the method does, so you could write all the code in a new method with an expressive name and call this new method within the Put. Just for having one expressive method name you should already notice an improvement in the code.

• If it is difficult to find a method name that reveals exactly the intent of this code block, then the method may be doing too many things, and then you break the code into new methods - each method needs to have a revealing name, and the code should do nothing less than the one indicated by its name.

• You really need to turn into Badrequest any exceptions that might occur? I believe that the ideal was there remove this exception treatment. An exception not provided for there should simply break the system. An exception generic treatment, outside this code block, could show the user a generic message and log the actual exception. A real message of an unforeseen exception may even reveal to the user more than you would like about your system architecture.

• "Looking quickly I can’t tell what the code is doing". A part of this problem should be solved with a good method name. The method name should say what. Already the as is another part of the problem and can be solved with a change of concept: you don’t have to understand how a code works by looking quickly! Source code is to be read - it’s not a drawing, it’s a text.

Be willing to read a code with dedication, as if it were an essay, as if it told a story, because that’s how it should be written, telling a story.

It should be written very succinctly and avoiding the technical details that do not concern the story, should avoid deviating from the main story, should be short if possible, and should still tell a story.