Снижение цикломатической сложности, не затрагивая бизнес-логику

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

Этот метод имеет много точек выхода и имеет цикломатические Сложность . Наши лучшие практики говорят, что он не должен превышать .

• Это из-за нескольких IFs?

• Что я могу сделать, чтобы реорганизовать это так, чтобы у него было меньше точек выхода?

Заранее спасибо.

Answer 1

---

Это резюме моих комментариев под вопрос выше

---

Наши лучшие практики говорят, что это не должно быть больше 5.

"5" звуки немного ниже. nDepend и Microsoft рекомендуют «30» и «25» соответственно.

NDepend:

Методы где CC выше, чем 15, трудно понять и поддержать. Методы, где CC выше, чем 30 чрезвычайно сложны и должны быть разделены на более мелкие методами (за исключением, если они автоматически генерируются с помощью инструмента)

Microsoft:

цикломатической сложности = число ребер - количество узлов + 1 , где узел представляет точку логической ветви, а край представляет собой линию между узлами. Правила сообщает о нарушении, когда цикломатические сложности более 25.

OP:

"Является ли это из-за несколько МСФА" -

да и ||

Что я могу сделать, чтобы реорганизовать это так, чтобы у него было меньше точек выхода?

Простой способ просто взять метод и «split into smaller methods». то есть вместо всего if s одним способом переместите некоторые из ваших логических схем if в один или несколько методов, каждый из которых вызывает другой.

например.

class Class1 
{ 
    class Hobbit 
    { 

    } 

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

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

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

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

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

     } 
    } 
} 

Может быть улучшен путем перемещения самой внутренней группы if с в отдельный метод:

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-подобных веб-вызовов, но используете ViewModels. Как это работает? Я всегда привязывал ViewModels к пользовательскому интерфейсу, а не к сети.