3
\$\begingroup\$

I am using ASP.NET core entity framework. I am using an input submit button to call a method that has a variable attached to it. I am wondering if there is a way to improve my code.

Here is my .cshtml code:

<form action="/DeleteItem/@item.Id" method="post">
        <td><input type="submit" name="submit" value="Delete"/></td>
</form> 

Here is the method it calls:

    [HttpPost]
    [Route("deleteItem/{id}")]
    public IActionResult DeleteItem(int id)
    {
        var temp = _context.Auction.Where(x => x.Id == id).SingleOrDefault();
        _context.Auction.Remove(temp);
        _context.SaveChanges();
        return RedirectToAction("Current", "Home");
    }
\$\endgroup\$
4
\$\begingroup\$

You might want to call your variable temp something like productToRemove

Single / SingleOrDefault both have an overload which accepts predicate you can get rid of the .Where() call with a single Single call:

var productToRemove = _context.Auction.SingleOrDefault(x => x.Id == id);

You are using SingleOrDefault but you are 100% sure that it will find a product and thus you are not performing a null check (it wont cause an exception even if the value is null). You should consider using simply .Single() in that case. Else you should add the respective null check and return as soon as you see there is no item to be removed.

Lastly you should use Single / SingleOrDefault only if you want to verify that there is exactly 1 object matching your predicate in your collection and throw some exceptions if there are multiple, if you don't care how much objects match your predicate in your collection or if you are sure there is always exactly one, you should consider using First / FirstOrDefault to get some performance.

\$\endgroup\$
1
\$\begingroup\$

In addition to the excellent answer from denis add a null check

if (temp != null)
{
    _context.Auction.Remove(temp);
    _context.SaveChanges();
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.