Need advice before I start with a bad habit

Question

I have a controller called AuctionsController. In it, I have actions called Index () and AuctionCategoryListing ():

//Used for displaying all auctions. public ActionResult Index() { AuctionRepository auctionRepo = new AuctionRepository(); var auctions = auctionRepo.FindAllAuctions(); return View(auctions); } //Used for displaying auctions for a single category. public ActionResult AuctionCategoryListing(string categoryName) { AuctionRepository auctionRepo = new AuctionRepository(); var auctions = auctionRepo.FindAllAuctions() .Where(c => c.Subcategory.Category.Name == categoryName); return View("Index", auctions); } 

As you can tell, they both invoke the same view (this action is called “invoke view. What is this name?).

 @model IEnumerable<Cumavi.Models.Auction> @{ ViewBag.Title = "Index"; } <h2>Index</h2> <p> @Html.ActionLink("Create New", "Create") </p> <table> <tr> <th></th> <th> IDSubcategory </th> <th> IDCity </th> <th> IDPerson </th> <th> Title </th> <th> TextBody </th> <th> ContactNumber </th> <th> AskingPrice </th> <th> AddressDirection </th> <th> LatestUpdateDate </th> <th> VisitCount </th> </tr> @foreach (var item in Model) { <tr> <td> @Html.ActionLink("Edit", "Edit", new { id=item.ID }) | @Html.ActionLink("Details", "Details", new { id=item.ID }) | @Html.ActionLink("Delete", "Delete", new { id=item.ID }) </td> <td> @item.IDSubcategory </td> <td> @item.IDCity </td> <td> @item.IDPerson </td> <td> @item.Title </td> <td> @item.TextBody </td> <td> @item.ContactNumber </td> <td> @String.Format("{0:F}", item.AskingPrice) </td> <td> @item.AddressDirection </td> <td> @String.Format("{0:g}", item.LatestUpdateDate) </td> <td> @item.VisitCount </td> </tr> } </table> 

They both inherit from the same Model.

My question is: am I doing everything right? Or is it just a hack that I managed to scrape off. Help me before I find out a bad habit.

Answer 1

I would change this to:

 public ActionResult Index(string categoryName) { AuctionRepository auctionRepo = new AuctionRepository(); var auctions=auctionRepo.FindAllAuctions(); if (!string.IsNullOrEmpty(categoryName)) { auctions = auctions.Where(c => c.Subcategory.Category.Name == categoryName); } return View(auctions); } 

Your route may look like this:

  context.MapRoute( "auction_defalt", "Auction/{categoryName}", new { controller="Auction", action = "Index", categoryName = UrlParameter.Optional } 

Since the actions are so similar, I see no reason to separate them.

Answer 2

Like any ASP.NET MVC infrastructure, you get many opportunities to shoot in the foot. Without forethought, reusing controller actions, viewing models and views can quickly become a nightmare for maintenance. Not to mention the fact that without a similar consideration, your routes will become difficult to tie together.

Following the principles of a convention on configuration, you can solve your problem using separate actions, but reusing a partial view. For me, the AuctionsController index must be responsible for listing all auctions in the system. I would not call the action of my category AuctionCategoryListing , but would instead call it simply Category . Thanks to the agreement, this has a good effect for laying routes like:

• site.com/auctions/ for the index
• site.com/auctions/category/CATEGORYNAME for the category.

The route is easily understood by the user and it is easy for you to understand what everyone is doing. (At this point, Omar provides a good suggestion in his answer so that your repository handles pagination, filtering, etc.)

For each action that you must return, you have several options. My preference would be to return individual views, each of which contains a link to a common partial. This gives you the flexibility to create different views around the partial, but allows reuse of the part that is shared.

Further reading that may help:

• One ViewModel per View
• ASP.NET MVC terminology disables me - why "ViewModel"?
• When should I use partial views in asp.net mvc?

Answer 3

You should branch out somewhere, so this is probably the preferred question.

The way I will handle it is to have a single method and to have a name as the category name. Since the strings are null, if one is not specified, it will be zero. My single-action method would probably look something like this:

 public ActionResult Index(string categoryName) { AuctionRepository auctionRepo = new AuctionRepository(); var auctions = auctionRepo.FindAllAuctions(); if(String.IsNullOrEmpty(categoryName) == false) auctions = auctions.Where(c => c.Subcategory.Category.Name == categoryName); return View(auctions); } 

Answer 4

I would rather have the repo perform functions such as filtering and pagination for performance and DRY concepts

 public ActionResult Index(string categoryName) { AuctionRepository auctionRepo = new AuctionRepository(); //Let the Repo Handle things like filtering and pagination, avoiding performance issues var auctions = auctionRepo.FindAllAuctions(categoryName); return View(auctions); } 

DAL should be responsible for these tasks.

Answer 5

This is an MVC function that the view and controller are independent. You can use the same or partial representations for the same thing in the same way, if something, I would say, is good, because you write reusable code and use it.