降低圈複雜度，而不會影響業務邏輯

Question

考慮這個方法：

public ActionResult DoSomeAction(ViewModel viewModel) 
{ 
    try 
    { 
     if (!CheckCondition1(viewModel)) 
      return Json(new {result = "Can not process"}); 

     if (CheckCondition2(viewModel)) 
     { 
      return Json(new { result = false, moreInfo = "Some info" }); 
     } 

     var studentObject = _helper.GetStudent(viewModel, false); 

     if (viewModel.ViewType == UpdateType.AllowAll) 
     { 
      studentObject = _helper.ReturnstudentObject(viewModel, false); 
     } 
     else 
     { 
      studentObject.CourseType = ALLOW_ALL; 
      studentObject.StartDate = DateTime.UtcNow.ToShortDateString(); 
     } 

     if (studentObject.CourseType == ALLOW_UPDATES) 
     { 
      var schedule = GetSchedules(); 

      if (schedule == null || !schedule.Any()) 
      { 
       return Json(new { result = NO_SCHEDULES }); 
      } 

      _manager.AddSchedule(schedule); 
     } 
     else 
     { 
      _manager.AllowAllCourses(studentObject.Id); 
     } 

     _manager.Upsert(studentObject); 

     return Json(new { result = true }); 
    } 
    catch (Exception e) 
    { 
     // logging code 
    } 
} 

這種方法有許多出口點，並具有一個圈複雜度。 我們的最佳實踐表示，它不應該大於。

• 是因爲有多個IF？

• 我能做些什麼來重構這個，使它有更少的出口點？

在此先感謝。

Answer 1

---

這是問題下面我上述評論

---

總結我們的最佳實踐說，這不應該是大於5

「5」的聲音有點低。 nDepend和微軟分別推薦「30」和「25」。

NDepend的：

方法，其中CC高於15是很難理解和維護。方法，其中CC高於30是極其複雜的，並應被劃分成更小的方法（除了當它們存在自動工具生成）

微軟：

圈複雜=邊緣的數量 - 節點的數目+ 1 其中節點表示邏輯分支點並且邊緣表示節點之間的線。 規則舉報違反時，圈複雜度超過25

OP：

「是因爲多個國際單項體育聯合會的」 -

是和||

我能做些什麼來重構這個，使它有更少的出口點？

一個簡單的方法是隻取方法和「split into smaller methods」。即用一種方法代替所有的ifs，將一些if邏輯移入一個或多個方法，每個方法調用另一個。

例如

void Foo(object person) 
    { 
     if (...) 
     { 
       // ... 

     } 
     else if (...) 
     { 
       // ... 
     } 

     if (x == 1 && person is Hobbit) 
     { 
      DoHobbitStuff(); 
     } 
    } 

    void DoHobbitStuff() 
    { 
     if (...) 
     { 
      // ... 
     } 

     if (...) 
     { 
      // ... 
     } 

     if (...) 
     { 
      // ... 
     } 
    } 

然而，在「5」我不相信你的代碼需要重構爲目的：

class Class1 
{ 
    class Hobbit 
    { 

    } 

    void Foo(object person) 
    { 
     if (...) 
     { 
       // ... 

     } 
     else if (...) 
     { 
       // ... 
     } 

     if (x == 1 && person is Hobbit) 
     { 
      if (...) 
      { 
       // ... 
      } 

      if (...) 
      { 
       // ... 
      } 

      if (...) 
      { 
       // ... 
      } 

     } 
    } 
} 

可以通過移動最內部組if s轉換爲單獨的方法來改善的CC減少。

我能做些什麼來重構這個，使它有更少的出口點？

據nDepend，出口點如return不計：

以下表達式不計算CC計算：

其他|做|開關|嘗試|使用|扔|終於|返回|對象創建|方法調用|現場訪問

Answer 2

看看你的代碼，顯然你的高迴圈複雜性和艱難的重構方式是壞設計（例如代碼氣味）的指標。讓我們回顧一下。

_helper 
_manager 

什麼是這些東西？爲什麼他們有這樣模棱兩可的名字？如果你找不到比這些更合適的名稱，這意味着你關注的問題是錯誤的。

_helper.GetStudent(viewModel, false); 
_helper.ReturnstudentObject(viewModel, false); 

我甚至無法想象如何能這些方法的工作。一些泛型助手如何知道如何從泛型ViewModel中獲得「學生」？這兩種方法有什麼區別？

var studentObject = _helper.GetStudent(viewModel, false); 

    if (viewModel.ViewType == UpdateType.AllowAll) 
    { 
     studentObject = _helper.ReturnstudentObject(viewModel, false); 
    } 
    else 
    { 
     studentObject.CourseType = ALLOW_ALL; 
     studentObject.StartDate = DateTime.UtcNow.ToShortDateString(); 
    } 

這確實看起來好像它應該是視圖模型的一部分。你正在根據ViewModel的內部狀態做出決定，只有ViewModel應該被允許。

_manager.Upsert(studentObject); 

是， 「UpdateOrInsert」？這是一個奇怪的命名約定。

令我困惑的另一件事是，您似乎在使用類似MVC的Web調用實現，但您使用的是ViewModels。這甚至如何工作？我總是將ViewModel與UI綁定，而不是使用Web。