Dependency Injection With Unity in MVC 5 without repository and unit of work

Question

I want to use dependency injection with Unity in my application. I am not following repository pattern and unit-of-work (and don't want to). I also have ViewModel in my controller action method. Please review my code and add your comments.

public class ProductCatagoryController : Controller
{
 private dataBaseContext _context;
 public ProductCatagoryController(IdataBaseContext context)
 {
 this._context = context as dataBaseContext;
 }
 [HttpPost]
 [ValidateAntiForgeryToken]
 public ActionResult addNewCategory([Bind(Include = "catName,desc")]RegisterNewCatagoryViewModel RegisterNewCatagoryViewModel)
 {
 if (ModelState.IsValid)
 {
 try
 {
 Mapper.CreateMap<RegisterNewCatagoryViewModel, Category>();
 Category category = Mapper.Map<Category>(RegisterNewCatagoryViewModel);
 category.catID = Guid.NewGuid().ToString();
 category.date_from = DateTime.Now;
 category.active = true;
 _context.Categories.Add(category);
 _context.SaveChanges();
 }
 catch (Exception e)
 {
 loggerElmah.logElmah(e, "Adding New Category Failed");
 }
 }
 return View();
 }
 [HttpGet]
 public ActionResult addNewCategory()
 {
 return View();
 }
 // GET: ProductCatagory
 public ActionResult Index()
 {
 return View();
 }
}

In addNewCategory I am passing an object of a concrete class object, but I want to pass an interface of the viewmodel and bind it with actual implementation in the run-time through unity. How can I do this?

Also, can someone guide me on writing unit-tests for the above code?

• Jamal: The last two questions are off-topic as we only review the working code given to us.

Answer 1

Naming

• Based on the naming guidelines methods should be named using PascalCasing casing.
  addNewCategory -> AddNewCategory

• Based on the same guideline classes should be named using PascalCasing casing.
  dataBaseContext -> DataBaseContext and therefor IdataBaseContext -> IDataBaseContext

General

You are injecting an interface in the constructor, but then you cast it to the concrete implementation. If you don't need any methods/properties of the concrete implementation, you should stick to the interface.

private IDataBaseContext _context;
public ProductCatagoryController(IDataBaseContext context)
{
 this._context = context;
}

This will make the ProductCatagoryController class independent of the concrete DatabaseContext object.

For the case that ModelState.IsValid is false, how will the consumer of this method see, that no category is added ?
If an exception occurs, the consumer of this method will only see it, if the log file is parsed. Returning an Exception view would do better.

• koushik: if i don't cast the interface object of DbContext then how will i do the SaveChanges(). cause the interface does not contains the SaveChanges() ? I don't want to new-ing objects in my controller.
• Heslacher: Then, why do you inject the interface instead of simply passing the concrete implementation ?
• koushik: i just don't want to use new operator to create objects. also i want to have my db calls in the controller itself. I dont know if my approach is right.
• ChrisWue: @user3132179 If you inject an interface then you should only use what the interface provides. If that is not enough and your controller depends on a concrete implementation then it should be getting the concrete implementation injected or the interface needs to be expanded. There absolutely no point to inject an interface and then cast it to a concrete implementation - this defeats the whole purpose of injecting the interface.