降低圈复杂度，而不会影响业务逻辑

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。