在不影响业务逻辑的情况下降低圈复杂度

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
    }
}

此方法有许多退出点，并且具有 8 的圈复杂度。 我们的最佳实践表明它不应大于 5。

• 是不是因为多个IF？

• 我可以做些什么来重构它以减少退出点？

提前致谢。

Answer 1

---

这是我对上述问题下的cmets的总结

---

我们的最佳实践表明它不应大于 5。

“5”听起来有点低。 nDepend 和 Microsoft 分别推荐“30”和“25”。

n依赖：

CC 高于 15 的方法难以理解和维护。 CC高于30的方法非常复杂，应该拆分成更小的方法（除非它们是由工具自动生成的）

微软：

圈复杂度=边数-节点数+1 其中一个节点代表一个逻辑分支点，一条边代表节点之间的一条线。 当圈复杂度大于 25 时，规则会报告违规。

操作：

“是不是因为多个IF”-

是的，||

我可以做些什么来重构它以减少退出点？

一个简单的方法是只使用方法和“split into smaller methods”。即，不是将所有ifs 放在一个方法中，而是将您的一些if 逻辑移动到一个或多个方法中，每个方法调用另一个。

例如

class Class1
{
    class  Hobbit
    {
         
    }

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

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

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

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

        }
    }
}

可以通过将最里面的ifs 组移动到一个单独的方法中来改进：

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

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

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

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

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

但是，在“5”处，我认为您的代码不需要重构以减少 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 的内部状态做出决策，这应该只允许 ViewModel 做。

_manager.Upsert(studentObject);

那是“UpdateOrInsert”吗？这是一些奇怪的命名约定。

让我感到困惑的另一件事是，您似乎在使用类似 MVC 的 Web 调用实现，但您使用的是 ViewModels。这甚至是如何工作的？我总是将 ViewModel 与 UI 联系起来，而不是与 Web 联系。