HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

User can add and delete customers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
candeleteuserandcustomersadd

Problem

I am using asp.net core entitiy framework. The page displays the customers. The user can add new customers or delete existing ones. I am looking for ways to improve my code.

Here is my cshtml page

@Html.Partial("Index")
@model ecommerce.Models.Users
Add a new customer
 
        
        Name
        
        Add
        
        

@{
        if(ViewBag.NotUnique!=null)
        {
                Name is alread in DB
        }

        if(ViewBag.Empty!=null)
        {
                Name is empty
        }
         
                
                        Customer Name
                        Created Date
                        Actions
                
        @if(ViewBag.Users != null)
        {
                foreach(var user in ViewBag.Users)
                {
                        
                                @user.name
                                @user.created_at.ToString("MMM")  @user.created_at.ToString("dd") @user.created_at.ToString("yyyy")

                                  
                                        
                                   

                        

                }
        }
         
}


Here is my cs file

```
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using ecommerce.Models;
using ecommerce.Controllers;
using System.Linq;

namespace ecommerce.Controllers
{
public class CustomerController : Controller
{
// GET: /Home/

private YourContext _context;

public CustomerController(YourContext context)
{
_context = context;//connection for my db
}

[HttpGet]
[Route("customers")]
public IActionResult Customers()
{
ViewBag.Users = _context.Users.AsEnumerable();
return View();
}

[HttpPost]
[Route("deleteCustomer/{id}")]
public IActionResult DeleteCustomer(int id)
{
var temp = _c

Solution

-
You don't need to enumerate twice here

var temp = _context.Users.AsEnumerable().ToList();


You can just call .ToList().

-
SingleOrDefault() also accepts predicates so the first .Where() call is unnecessary, you can just do it like this :

Your code :

var temp = _context.Users.Where(x => x.id == id).SingleOrDefault();


Can become :

var temp = _context.Users.SingleOrDefault(x => x.id == id);


-
I don't see any reason to call .ToList() here :

var temp = _context.Users.ToList();
    if (temp.Any(x => x.name == name))
    {
        ViewBag.NotUnique = true;
        return RedirectToAction("Customers");
    }


You are not performing any List specific operations, (I assume Users is IEnumerable), you are good with a pure IEnumerable here. In fact, you don't even need the temp variable, you can just directly access _context.Users.

-
You can have just ActionResult instead of IActionResult as a return type for your methods.

Design

-
Usually asp.net-mvc projects work with multiple DTO's (Data Transfer Object), which are almost identical to your actual model, but you can remove some things and add others, so that you can define the way your data will be passed around your application. Often the ViewModel is mapped in a way to it's respective DTO, to ease the creation of both objects.

Even tho simple projects probably wont need any additional DTO's.

-
Your models should have proper validation instead of some check against null in your controller, which should only verify that the validation is successful. For example DataAnnotations offers quite some variety of pre-written validation attributes. You can of course extend this with your own, but it already has the core things you might need. You can find examples here

Code Snippets

var temp = _context.Users.AsEnumerable().ToList();
var temp = _context.Users.Where(x => x.id == id).SingleOrDefault();
var temp = _context.Users.SingleOrDefault(x => x.id == id);
var temp = _context.Users.ToList();
    if (temp.Any(x => x.name == name))
    {
        ViewBag.NotUnique = true;
        return RedirectToAction("Customers");
    }

Context

StackExchange Code Review Q#152515, answer score: 6

Revisions (0)

No revisions yet.