2009-03-25 14 views
4

J'ai plusieurs méthodes dans un de mes contrôleurs qui fait cela:Comment puis-je utiliser le principe DRY dans ASP.NET MVC pour refactoriser ce code?

ViewData["Customers"] = LoadCustomers(); 
ViewData["Employees"] = LoadEmployees(); 
ViewData["Statuses"] = LoadStatuses(); 
etc...... 

Voici LoadCustomers(), mais LoadEmployees, LoadStatuses et tous les autres sont pratiquement exactement la même logique:

private static SelectList LoadCustomers() 
    { 
     IList<Customer> customers; 
     try 
     { 
      IServiceCallService scService = new ServiceCallService(); 
      customers = scService.GetCustomers(); 
      Customer c = new Customer 
      { 
       ID = "", 
       Name = "-- Select a Facility --" 
      }; 
      customers.Insert(0, c); 
     } 
     catch 
     { 
      customers = new List<Customer>(); 
      Customer c = new Customer 
      { 
       ID = "", 
       Name = "-- No Facilities on File --" 
      }; 
      customers.Insert(0, c); 
     } 

     return new SelectList(customers, "ID", "Name"); 
    } 

Comment puis-je mieux écrire ce code, donc je n'ai pas besoin d'une nouvelle méthode chaque fois que j'ajoute une nouvelle liste de sélection?

Répondre

5

Cela ressemble à cela pourrait être un bon candidat pour les médicaments génériques:

private static SelectList LoadItems<T>() where T : new, ... 
{            // Add any additional interfaces 
               // that need to be supported by T 
               // for your Load method to work, 
               // as appropriate. 
    IList<T> items; 
    try 
    { 
     IServiceCallService scService = new ServiceCallService(); 
     results = scService.Get<T>(); // You'll need to replace GetCustomers() with 
             // a generic Get<T> method. 

     // ... 
    } 
    catch   // Really needed? What are you trying to catch here? (This catches 
    {    // everything silently. I suspect this is overkill.) 
     // ... 
    } 

    return new SelectList(items, "ID", "Name"); 
} 
+0

Vous devriez également vous débarrasser de ce bloc try/catch. Il va manger toutes les exceptions, peu importe leur gravité. Si vous obtenez une erreur catastrophique, alors vous ne voulez pas * continuer *! –

0

Vous pouvez également essayer une approche plus fonctionnelle.

public IList<T> Load<T>(Func<IList<T>> getList, T prependItem) 
{ 
    var list = getList(); 
    list.Insert(0, prependItem); 
    return list; 
} 

Utilisation:

var prependItem = new Customer { ID = "", Name = "-- Select a Facility --" }; 
ViewData["Customers"] = new SelectList(
    Load(new ServiceCallService().GetCustomers(), prependItem), 
    "ID", "Name"); 

Cette approche sépare également la construction de la liste des détails de ce qui est construit - un SelectList.

0

J'irais encore plus loin et écrire ce dans le code du contrôleur:

ViewData["Customers"] = new SelectList(
     new ServiceCallService().GetCustomers(), 
     "ID","Name") 

et cette perspective

<%= Html.DropDownList("Customers", 
    ((SelectList)ViewData["Customers"]).Count() > 0 ? 
    "-- Select a Facility --" : "-- No Facilities on File --") %>