tb517
tb517

Reputation: 437

ASP.NET MVC / APIs - DELETE method not returning correct response

I have a DELETE and PUT method which contain the same code to get the list of Ids from the database and check to see if they exist before performing the action:

try
{
    var ids =
        (await _testTableTbRepository.GetAllAsync())
        .Select(_ => new TestTableTbModel { Id = _.Id }).OfType<int>().ToList();

    if (!ids.Contains(model.Id))
    {
        return StatusCode(400, "Id: " + model.Id + " does not exist");
    }

    await _testTableTbRepository.DeleteAsync(model.Id);
    return Ok(model.Id);
}
catch (Exception e)
{
    return StatusCode(500, e);
}

My thought is that the conversion of Ids to integers is not working as it should, so it keeps getting caught at the if-statement and returning a 400 status. Does anyone see anything wrong with this? Is there a better way to write this?

Upvotes: 0

Views: 1117

Answers (2)

Fran
Fran

Reputation: 6530

I'm not sure how _testTableRepository is implemented, but I making the assumption that it's just wrapping EF functions.

var exists = await _testTableRepository.FindAsync(model.Id);

if (!exists)
 return NotFound();

_testTableRepository.Delete(Model.Id);

return Ok(model.Id);

Don't bother with the try catch block. Most exceptions will automatically be converted to a 500 response code. If that's all your try/catch is doing, just let the framework handle it.

It's also easy enough to check by id for the record you want. Like I said about i'm making the assumption that your repository is just forwarding on the call to an EF DbContext, which will have a FindAsync() method on it.

Upvotes: 0

Hey24sheep
Hey24sheep

Reputation: 1202

Why all this hassle? Let DB Help you Use DBException instead.

This way you wont load a huge db in your memory and save one giant call plus all that select and contain checks.

try
{
    await _testTableTbRepository.DeleteAsync(model.Id);
    return Ok(model.Id);
}
catch(DbException e)
{
   return StatusCode(404, "Id: " + model.Id + " does not exist");
}
catch (Exception e)
{
    return StatusCode(500, e);
}

Thanks @Fran for pointing out 400 -> 404

Upvotes: 2

Related Questions