Программный код, будучи по факту виртуальной сущностью не может иметь запах в прямом смысле этого слова. Однако, термин “запах кода” (code smell) некоторое время назад был введен Кентом Беком и популяризирован книгой Мартина Фаулера о рефакторинге (Refactoring: Improving the Design of Existing Code).

В русскоязычном переводе можно встретить “код с душком”. Такой перевод явно говорит о том, что речь идет о чем-то не слишком хорошем и для того, чтобы понять, что же такое code smell, рассмотрим несколько примеров.

Метод слишком длинный

Иногда метод содержит слишком много строк кода, например, если он содержит более 50 строк, более вероятно, что он слишком длинный. Хотя, на самом деле проблема даже не в количестве строк, а в том, что этот код делает. Так, если метод выполняет более одной функции, вам следует рассмотреть возможность его изменения, потому что методы меньшего размера легче понять. Лучшее решение - перенести некоторую логику в отдельный метод.

Например, взгляните на следующий метод:

//list of available smartphone results

List<string> smartphones = new List<string>()

    {

    "Samsung Galaxy S20",

    "Pixel 2",

    "Pixel 3",

    "Pixel 4",

    "iPhone XR",

    "iPhone 12",

    "iPhone 12 Pro",

    "iPhone 12 Pro Max"

    };

 

//long method that we will refactor

public void PerformSearch()

{

    bool continueSearch = true;

 

    while (continueSearch)

    {

        //user enters the term

        Console.Write("Search for smartphone: ");

        string keyword = Console.ReadLine();

 

        var results = smartphones

            .Where(phone => phone

                                .ToLower()

                                .Contains(keyword.ToLower()));

        //if there are resuls, they are displayed in the output

        if (results != null)

        {

            Console.WriteLine("Here are the matched results.\n");

 

            foreach (var result in results)

            {

                Console.WriteLine(result);

            }

        }

        else

        {

            Console.WriteLine("No results found.");

        }

 

        //this asks user if he wants to search again

        //valid responses are Y and N

        //the program stops if the answer is N

        string continueSearchResponse;

        do

        {

            Console.Write("\nMake another search (y/n)?: ");

            continueSearchResponse = Console.ReadLine();

 

            if (continueSearchResponse.ToLower() == "n")

            {

                continueSearch = false;

                break;

            }

            if (continueSearchResponse.ToLower() != "y")

            {

                Console.WriteLine("Invalid response.");

            }

 

        } while (continueSearchResponse.ToLower() != "n"

                 && continueSearchResponse.ToLower() != "y");

    }

 

    Console.Write("Thanks for searching!");

}

Трудно понять, что он делает, не потратив некоторое время на изучение кода. Взгляните на тот же метод, но улучшенный с помощью рефакторинга Extract Method:

public void PerformSearch()

{

    bool continueSearch = true;

 

    while (continueSearch)

    {

        SearchForSmartphones();

 

        continueSearch = ShouldContinueWithSearch(continueSearch);

    }

 

    Console.Write("Thanks for searching!");

}

 

Это проще и легче для понимания. Если вам нужно узнать о специфике кода, вам следует перейти к методам Search For Smart phones и ShouldContinueWithSearch.

Длинные методы - это тот самый код с душком, который вам нужно реорганизовать. Длинные методы могут затруднить добавление новых функций или модификацию существующих. Прежде чем вы попытаетесь добавить новую функцию, сначала вам нужно потратить некоторое время на ознакомление с этим методом. И если метод длинный, вам нужно потратить больше времени на изучение того, что делает этот метод. Длинные методы также более сложны для понимания, их сложнее тестировать и сложнее использовать повторно.

Класс бога

Еще один пример кода с душком, это класс с большим количеством строк кода. Некоторые разработчики называют этот тип классов “классом бога”. Этот класс выполняет множество функций и содержит тысячи строк кода.

Например:

public class TeacherHomeController : Controller

{

    public ActionResult Index() { }

    public ActionResult QuizList(int? pageId, int? course_id, int? category_id, string title) { }

    public ActionResult QuizDetail(int? quiz_id) { }

    public ActionResult StudentResultList(int? pageId, int? quiz_id) { }

    public ActionResult EditQuizDetail(quiz FormData) { }

    public ActionResult AddQuiz() { }

    public ActionResult AddQuiz(quiz FormData) { }

    public ActionResult DeleteQuiz(quiz FormData) { }

    public ActionResult StudentResultDetail(int? quiz_id, int? student_id) { }

    public ActionResult AddQuestion(quiz_questions FormData) { }

    public ActionResult EditQuestion(quiz_questions FormData) { }

    public ActionResult DeleteQuestion(quiz_questions FormData) { }

    public ActionResult Courses(int? pageId, string title, int? status, int? course_category_id) { }

    public ActionResult StudentsList(int? pageId, string user_name, string email, int? student_id) { }

    public ActionResult AssignmentList(int? pageId, int? course_id, string title) { }

    public ActionResult DeleteAssignment(assignments FormData) { }

    public ActionResult AddAssignment() { }

    public ActionResult AddAssignment(assignments FormData) { }

    public ActionResult AssignmentDetail(int? assignment_id) { }

    public ActionResult EditAssignmentDetail(assignments FormData) { }

    public ActionResult StudentAssignmentResultList(int? pageId, int? assignment_id) { }

    public ActionResult StudentAssignmentResultDetail(int? assign_answers_id, int? student_id) { }

    public ActionResult GiveAssignmentMarkToStudent(assignment_answers FormData) { }

    public ActionResult StudentListInCourses(int? pageId, int? course_id, string user_name, int? student_id) { }

}

Класс содержит 24 метода и выполняет большую работу. Чем больше класс, тем больше кода он будет содержать, и тем больше задач он будет выполнять. Но когда они начинают делать больше, их становится трудно поддерживать.

Поэтому лучше, когда классы делают только что-то одно. Это также известно как принцип единой ответственности. Хотя многие факторы могут привести к увеличению размера класса, одним из наиболее распространенных факторов являются статические методы. Статические методы вызываются из разных частей вашего решения. И именно поэтому, когда вам нужно добавить новый метод, вы добавляете новый статический метод в этот “класс бога”.

Такая архитектура кода может быть признаком плохого проектирования системы, и вам следует провести рефакторинг класса. Возможно стоит перенести некоторые задачи в другой класс. Если класс находится в иерархии наследования, посмотрите, можете ли вы переместить какой-либо код в базовый класс или подкласс.

Когда все двоится

А это пример кода с душком пожалуй можно назвать наиболее распространенным. Дублирующийся код можно встретить программах, написанных студентами младших курсов технических вузов. Однако, даже профессиональные программисты часто дублируют код. Например, вы находите нужное вам решение в существующем коде, вставляете его в новое место и меняете несколько строк. И готово. Проблема решена.

Непонимание назначения дублированного кода может привести к возникновению более глубокой проблемы в приложении. Есть много причин, по которым дублирующийся код - это плохо. Во-первых, когда у вас есть дублирующийся код, вы рискуете привнести ту же ошибку в новое место. И когда у вас есть дублирующийся код, вы никогда по-настоящему не уверены, что нашли все ошибки в одном месте.

Далее, если вам нужно изменить какой-либо код, чтобы добавить новую функцию, но у вас есть такой же код в другом месте, вам также нужно не забыть зайти во все эти другие места и посмотреть, нужно ли вам вносить изменения там. И есть вероятность, что вы не вспомните, где находятся эти другие места в вашем коде. Наличие дублирующегося кода в вашей кодовой базе не улучшает удобство обслуживания приложения. В результате у нас возникает необходимость в рефакторинге копируемоего кода для того, чтобы избежать данных проблем.

Длинный список параметров

Еще один признак того, что метод делает слишком много это длинный список параметров. Что квалифицируется как длинный список параметров? Опять же, это зависит от контекста и кода, с которым вы работаете. Но если метод имеет более четырех параметров, вам следует начать задавать некоторые вопросы по этому поводу. Например, если у вас есть длинный список параметров, подобный этому:

            public void SaveHomeAddress(string name,

                                        string homeAddress,

                                        string country,

                                        string email,

                                        string fileLocation)

        {

        }

Подходящим рефакторингом здесь будет:

public void SaveHomeAddress(AddressDetails addressDetails)

Полезно упростить список параметров, сделав его короче и упорядочив параметры. Длинный список параметров может вызвать проблемы в виде путаницы и как следствие, более сложной и продолжительной отладки. Также сложнее вызвать метод из других мест, если у него есть пять, шесть или более различных параметров, которые вам нужно передать ему.

Что делать?

Мы рассмотрели несколько наиболее распространенных примеров кода с душком. Давайте посмотрим, что можно с этим сделать или не сделать. В простейшем случае вы можете не делать ничего – программа работает и ладно, сделаю рефакторинг когда-нибудь потом. Пагубность такого подхода состоит в том, что когда вам все-таки придется избавляться от не совсем правильного кода, вам придется потратить больше времени на рефакторинг.

Однако, иногда возможны ложные срабатывания. То есть, вы видите метод с множеством параметров, но понимаете, что разбивать его на методы меньших размеров не имеет особого смысла.

Заключение

Проблема кода с душком может показаться неочевидной, ведь код работает правильно, но это только пока. Когда вам нужно будет внести в него изменение, вы потратите больше времени чем ожидалось, потому что будете дольше разбираться в самом коде. И со временем правки в таком коде превратятся в ад. Так что лучшим решением будет рефакторинг такого кода.

А о том, как писать код правильно, вы можете узнать на курсах OTUS, которые ведут действующие эксперты отрасли.

Комментарии (5)


  1. delphinpro
    10.10.2023 15:12
    +26

    Ну если писать код через строчку, то он станет еще длиннее.
    Зачем это делать? Что за стиль такой странный?
    Если вам лично нравятся большие промежутки между строк – увеличьте в своем редакторе межстрочный интервал.


  1. nUser123
    10.10.2023 15:12
    +12

    Я вот думаю, не написать ли мне статью. Уже года два как думаю. И даже вроде есть о чем. А тут нате - раз и статья. Целых полчаса поди писалась. А то, что слабая, не соответствует ожиданию (из названия) - это плохо. Я могу сказать, что код пахнет, если много переменных из одной буквы, если слишком длинныные строки, слишком мноо буков... Можно бесконнчно перечислять признаки чего угодно и ни слова не сказать о сути. Вот и ствтья ваша - слвчайный набор признаков без грамма истинной причины пояаления душка в коде. Извините, но статья ради статьи - не статья.


  1. guryanov
    10.10.2023 15:12
    +9

    Ну вот по поводу списка параметров это прямо не рефакторинг, а вредительство. Было 5 явных параметров а стала структура с именем. То есть вместо того, чтобы просто прямо здесь посмотреть список параметров надо перейти в определение структуры и уже там посмотреть.

    Обычно такое происходит, когда линтер говорит, что функция слишком длинная и человек просто делит ей на 2-3 куска. Внезапно оказывается, что было много локальных переменных, которые нужны во всех функциях, приходится делать по 5-7 параметров в функциях, он опять запускает линтер и линтер опять ругается. И так как человеку уже хочется побыстрее запушить он просто меняет 5-7 параметров на структуру MyFunctionParams. И в результате вместо одной функции в 100 строк мы получаем 3 функции по 80 строк и 2 структуры по 5-7 полей. Так себе рефакторинг, уж лучше длинная функция.

    По-хорошему то надо сокращать количество параметров, но делать это другим способом глубоко проанализировать код и распутать там все использования переменных чтобы реально можно было свести к минимуму количество передаваемых сущностей/значений.


    1. Tiriet
      10.10.2023 15:12

      а есть всякие мерзкие библиотеки типа IMKL/AlgLib etc, где злые математики уже десятки лет не чураются в функции по десятку параметров передавать, и код у них там такой, что даже если ты его на зубок выучишь- то все равно невозможно понять, что он делает, не прочитав десяток специальных статей и пяток учебников. Так там за улучшение "запаха кода" можно знатно огрести- это как сыр с плесенью- он настолько воняет, что только истинные ценители могут понять, в чем там прелесть.

      -- ALGLIB -- Copyright 25.09.2021 by Bochkanov Sergey *************************************************************************) procedure sparsesolvesymmetricgmres(const a: Tsparsematrix; const isupper: Boolean; const b: TVector; const k: TALGLIBInteger; const epsf: Double; const maxits: TALGLIBInteger; out x: TVector; out rep: Tsparsesolverreport; const _xparams: UInt64 = 0);

      Двадцать лет люди кладут болт на вообще все принципы хорошего кода! КемелКейз? Информативные имена переменных? Самодокументируемый код? Да у нас функции по 200 строк- каждая третья! Правда, если я раскурил, как работает GMRES для решения разреженных симметричных систем (sparce symmetric matrix)- то код этой процедурки мне довольно понятен :-) и я понимаю, что или вот так- 200 строк и девять параметров, или лучше вообще никак, и любое предложение рефакторинга будет сразу же отклонено. А "глубокий анализ" этого кода показывает, что этот код- это просто код, который самым простым способом реализует сложный алгоритм. А алгоритм сложный просто потому что он сложный- ну вот так бывает. Причем, GMRES- это довольно простой алгоритм среди своего класса алгоритмов- там есть и понавороченнее- математики стараются :-).


      1. guryanov
        10.10.2023 15:12

        Ага, согласен, бывает код, который словами надо будет объяснять целый. И как его не разбивай и не украшай сильно понятнее он не станет.