如何重構此C＃代碼以使其更易於閱讀？

Question

這是我的控制器中的一個動作 - 在我的項目中，我的控制器中有類似的大方法。

我想學習把這些東西放在哪裏以及如何清理東西。我是新手，如果我看到一個很好的例子來說明如何改變我自己的代碼，那很可能會教我如何去做大量的代碼。

這裏是我的行動：

public ActionResult Index(string sortOrder, string currentFilter, string searchString, int? page) 
{ 
    ViewBag.CurrentSort = sortOrder; 
    ViewBag.TitleSortParm = String.IsNullOrEmpty(sortOrder) ? "Title desc" : ""; 
    ViewBag.CreditsSortParm = sortOrder == "Credits" ? "Credits desc" : "Credits"; 
    ViewBag.ElectiveSortParm = sortOrder == "Elective" ? "Elective desc" : "Elective"; 
    ViewBag.InstructorSortParm = sortOrder == "Instructor" ? "Instructor desc" : "Instructor"; 
    ViewBag.YearSortParm = sortOrder == "Year" ? "Year desc" : "Year"; 
    ViewBag.AttendingDaysSortParm = sortOrder == "AttendingDays" ? "AttendingDays desc" : "AttendingDays"; 
    ViewBag.AttendanceCapSortParm = sortOrder == "AttendanceCap" ? "AttendanceCap desc" : "AttendanceCap"; 
    ViewBag.StartDateSortParm = sortOrder == "StartDate" ? "StartDate desc" : "StartDate"; 
    ViewBag.LocationSortParm = sortOrder == "Location" ? "Location desc" : "Location"; 
    ViewBag.ParishSortParm = sortOrder == "Parish" ? "Parish desc" : "Parish"; 
    ViewBag.DescriptionSortParm = sortOrder == "Description" ? "Description desc" : "Description"; 
    ViewBag.ApprovedSortPArm = sortOrder == "Approved" ? "Approved desc" : "Approved"; 
    ViewBag.CompletedSortPArm = sortOrder == "Completed" ? "Completed desc" : "Completed"; 
    ViewBag.ArchivedSortPArm = sortOrder == "Archived" ? "Archived desc" : "Archived"; 

    if (Request.HttpMethod == "GET") 
    { 
     searchString = currentFilter; 
    } 
    else 
    { 
     page = 1; 
    } 
    ViewBag.CurrentFilter = searchString; 

    var courses = from s in db.Courses 
        select s; 
    if (!String.IsNullOrEmpty(searchString)) 
    { 
     courses = courses.Where(s => s.Title.ToUpper().Contains(searchString.ToUpper())); 
    } 
    switch (sortOrder) 
    { 
     case "Title desc": 
      courses = courses.OrderByDescending(s => s.Title); 
      break; 
     case "Credits": 
      courses = courses.OrderBy(s => s.Credits); 
      break; 
     case "Credits desc": 
      courses = courses.OrderByDescending(s => s.Credits); 
      break; 
     case "Elective": 
      courses = courses.OrderBy(s => s.Credits); 
      break; 
     case "Elective desc": 
      courses = courses.OrderByDescending(s => s.Credits); 
      break; 
     case "Instructor": 
      courses = courses.OrderBy(s => s.Instructor.LastName); 
      break; 
     case "Instructor desc": 
      courses = courses.OrderByDescending(s => s.Instructor.LastName); 
      break; 
     case "Year": 
      courses = courses.OrderBy(s => s.Year); 
      break; 
     case "Year desc": 
      courses = courses.OrderByDescending(s => s.Year); 
      break; 
     case "AttendingDays": 
      courses = courses.OrderBy(s => s.AttendingDays); 
      break; 
     case "AttendingDays desc": 
      courses = courses.OrderByDescending(s => s.AttendingDays); 
      break; 
     case "AttendanceCap": 
      courses = courses.OrderBy(s => s.AttendanceCap); 
      break; 
     case "AttendanceCap desc": 
      courses = courses.OrderByDescending(s => s.AttendanceCap); 
      break; 
     case "StartDate": 
      courses = courses.OrderBy(s => s.StartDate); 
      break; 
     case "StartDate desc": 
      courses = courses.OrderByDescending(s => s.StartDate); 
      break; 
     case "Location": 
      courses = courses.OrderBy(s => s.Location); 
      break; 
     case "Location desc": 
      courses = courses.OrderByDescending(s => s.Location); 
      break; 
     case "Parish": 
      courses = courses.OrderBy(s => s.Parish); 
      break; 
     case "Parish desc": 
      courses = courses.OrderByDescending(s => s.Parish); 
      break; 
     case "Description": 
      courses = courses.OrderBy(s => s.Description); 
      break; 
     case "Description desc": 
      courses = courses.OrderByDescending(s => s.Description); 
      break; 
     case "Approved": 
      courses = courses.OrderBy(s => s.Approved); 
      break; 
     case "Approved desc": 
      courses = courses.OrderByDescending(s => s.Approved); 
      break; 
     case "Completed": 
      courses = courses.OrderBy(s => s.Completed); 
      break; 
     case "Completed desc": 
      courses = courses.OrderByDescending(s => s.Completed); 
      break; 
     case "Archived": 
      courses = courses.OrderBy(s => s.Archived); 
      break; 
     case "Archived desc": 
      courses = courses.OrderByDescending(s => s.Archived); 
      break; 
     default: 
      courses = courses.OrderBy(s => s.Title); 
      break; 
    } 
    int pageSize = 4; 
    int pageNumber = (page ?? 1); 
    return View(courses.ToPagedList(pageNumber, pageSize)); 
} 

我應該怎麼做上面的代碼，以使其更具可讀性？我是否只是將其中的部分作爲單獨的方法移出並將它們移動到控制器的底部？我是否將這些方法放在另一個文件中並在此處引用？

請記住我正在學習和清晰度。

Answer 1

你如何改變代碼將是純粹的主觀，這是一個創造性的過程，你需要感受和努力成爲一個更好的開發者。

但是作爲一個僞代碼的例子，對象是讓你的方法變小。不，真的，甚至更小！

使函數名稱非常有意義，並確保它們告訴程序員該方法的意圖是什麼。這僅僅是一個結構的樣品：

public class Something { 

    public ActionResult Index() { 
     MakeCallToFunction(); 
     var something = GetSomething(); 
     var somethingElse = GetSomethingElse(something); 
     var model = new SomeViewModel(somethingElse); 
     return View(model); 
    } 

    private void MakeCallToFunction() { 
     ... 
    } 

    private string GetSomething() { 
     ... 
     return someVal; 
    } 

    private SomeObject GetSomethingElse(string something) { 
     ... 
     return someBigObject; 
    } 
} 

或者，它可能意味着你把所有這些小功能，使類

public class Something { 

    public ActionResult Index(int id) { 
     var model = new SomeRelatedStuff.Load(id); 
     return View(model); 
    } 
} 

private class SomeRelatedStuff { 

    public SomeRelatedStuff() 

    public static SomeRelatedStuff Load(int id) { 
     var something = GetSomething(); 
     var somethingElse = GetSomethingElse(something); 
     var model = new SomeViewModel(somethingElse); 
     MakeObject(); 
     return this; 
    } 

    private string GetSomething() { 
     ... 
     return someVal; 
    } 

    private SomeObject GetSomethingElse(string something) { 
     ... 
     return someBigObject; 
    } 

    private void MakeObject() { 
     ... 
    } 

} 

作爲一個側面說明，請請請去了羅伯特得到乾淨的代碼馬丁（鮑勃叔叔），本週末至少讀了前四章。然後再讀一遍。

Answer 2

首先，擺脫你的魔術弦。做一個排序順序的枚舉，像這樣：

public enum ActionSortOrder { 
    Credits, 
    Elective, 
    ... 
} 

ActionSortOrder order = ActionSortOrder.Credits; 

這比使用未定義的字符串更易於管理，而且如果它的功能需要一些解釋，你可以記錄枚舉成員。

其次重要的是，在方法的頂部線條有些混亂：

ViewBag.CreditsSortParm = sortOrder == "Credits" ? "Credits desc" : "Credits"; 

所以，如果你通過「信用」作爲排序方式，則默認爲降序排列？如果你不這樣做，它默認爲升序？爲什麼？爲什麼不給出升序/降序的選項（再次使用枚舉而不是魔術字符串）？爲什麼每次調用這個方法時都會保存所有這些排序順序？這些設置變量在哪裏使用？您只能使用此方法內傳遞的順序。

另外，不要擔心開關看起來很大。 Switch語句通常只用於避免一堆if-else語句。所以大多數開關在實踐中有不少案例。

瞭解更多關於您的代碼將有助於評估它。