Method with many if and return

Question

Method with many if and return

Navigation

#1 by (2 votes)

3

I think my code is very 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 a more elegant way?

c#

asked by anonymous 18.11.2016 / 02:51

1 answer

how to execute a specific button in a list of 5 identical buttons Problem with each jquery

score 2 · Accepted Answer

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 inside Put . Just by having an expressive method name you should already notice an improvement in the code.
If it is difficult to find a method name that exactly reveals the intent of this block of code, then the method may be doing too many things , and then you break the code into new methods - each method must have a revealing name, and the code should do nothing less than that indicated by its name.
Do you really need to turn BadRequest into any exception that might occur? I believe the ideal there was to remove this exception handling . An exception not provided for there should simply break the system. A generic exception handling, outside this block code, could show the user a generic message and log the actual exception. A real message from an unplanned exception might even tell the user more than you would like about your system architecture.
"Quickly looking I can not tell what the code is doing" . A part of this problem must be solved with a good method name. The method name should say what . Since as is another part of the problem and can be solved with a change of concept: You do not have to understand how a code works looking fast! 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 is how it should be written, telling a story.

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