Reputation: 437
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
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
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