Code smell: когда код плохо пахнет
Программный код, будучи по факту виртуальной сущностью не может иметь запах в прямом смысле этого слова. Однако, термин «запах кода» (code smell) некоторое время назад был введен Кентом Беком и популяризирован книгой Мартина Фаулера о рефакторинге (Refactoring: Improving the Design of Existing Code).
В русскоязычном переводе можно встретить «код с душком». Такой перевод явно говорит о том, что речь идет о чем-то не слишком хорошем и для того, чтобы понять, что же такое code smell, рассмотрим несколько примеров.
Метод слишком длинный
Иногда метод содержит слишком много строк кода, например, если он содержит более 50 строк, более вероятно, что он слишком длинный. Хотя, на самом деле проблема даже не в количестве строк, а в том, что этот код делает. Так, если метод выполняет более одной функции, вам следует рассмотреть возможность его изменения, потому что методы меньшего размера легче понять. Лучшее решение — перенести некоторую логику в отдельный метод.
Например, взгляните на следующий метод:
//list of available smartphone results
List smartphones = new List()
{
"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, которые ведут действующие эксперты отрасли.
