gadget59.ru практика

Рефакторинг и код-ревью · 5 раундов

Рефакторинг и код-ревью

Вы — тимлид в IT-компании. Вам на ревью прислали код, который «вроде работает». Но у него есть проблемы: плохие имена, дублирование, магические числа, отсутствие обработки ошибок. Ваша задача — провести код-ревью, объяснить, что не так, и переписать код так, чтобы он был понятным, безопасным и красивым.

             Код-ревью — это процесс, в котором разработчики смотрят чужой код и предлагают улучшения. Это не «придраться», а «сделать лучше».
        

Как работать:

Читайте код и находите проблемы.
Пишите свой разбор в поле для ответа.
Если нужно — нажмите «Подсказка».
В конце нажмите «Сохранить отчёт» — ответы скачаются в файл.

1 Раунд 1: «Студенческий калькулятор» Лёгкий

Коллега прислал код, который считает средний балл и выводит сообщение. Код работает, но написан «как получилось». Найдите как минимум 4 проблемы и объясните, как их исправить.

public class Calc { public static void main(String[] args) { double a = 4.5; double b = 3.0; double c = 5.0; double d = 2.5; double avg = (a + b + c + d) / 4; if (avg >= 4.5) { System.out.println("Отлично"); } else if (avg >= 3.5) { System.out.println("Хорошо"); } else if (avg >= 2.5) { System.out.println("Удовлетворительно"); } else { System.out.println("Плохо"); } } }

Что нужно сделать:

Найти проблемы: имена, магические числа, дублирование, отсутствие структуры.
Переписать код: вынести константы, использовать массив, разделить на методы.
Объяснить, почему так лучше.

Ваш разбор и улучшения

Подсказка. Обратите внимание на:

Имена: a, b, c, d — что это? Лучше mathGrade, physicsGrade и т.д.
Магические числа: 4.5, 3.5, 2.5 — вынести в константы EXCELLENT, GOOD, SATISFACTORY.
Дублирование: четыре переменные одного типа — просятся в массив.
Структура: всё в main — вынести в методы calculateAverage() и getGrade().

2 Раунд 2: «Обработка заказов» Лёгкий

Код считает стоимость заказа с учётом скидки. Найдите проблемы и перепишите.

public class Order { public static double total(double price, int count, boolean vip) { double result = price * count; if (vip) { result = result * 0.9; } if (result > 1000) { result = result * 0.95; } return result; } }

Что нужно сделать:

Найти проблемы: магические числа, отсутствие проверок, плохие имена.
Переписать: добавить проверки на некорректные значения, вынести константы.
Объяснить, почему проверки важны.

Ваш разбор и улучшения

Подсказка. Обратите внимание на:

Магические числа: 0.9, 0.95, 1000 — вынести в константы.
Проверки: что будет, если price < 0 или count < 0? Нужно выбрасывать IllegalArgumentException.
Имена: result — лучше totalPrice.
Документация: неплохо было бы добавить Javadoc.

3 Раунд 3: «Список студентов» Средний

Код хранит список студентов и выводит их. Есть несколько проблем, которые могут привести к ошибкам.

import java.util.ArrayList;\n import java.util.List;\n \n public class Group {\n private List<String> students = new ArrayList<>();\n \n public void add(String name) {\n if (name != null || !name.isEmpty()) {\n students.add(name);\n }\n }\n \n public List<String> getStudents() {\n return students;\n }\n \n public void print() {\n for (int i = 0; i <= students.size(); i++) {\n System.out.println(students.get(i));\n }\n }\n }

Что нужно сделать:

Найти логические ошибки: условие в add, цикл в print, возврат внутреннего списка.
Исправить каждую и объяснить, что произойдёт, если оставить как есть.
Предложить, как сделать код безопаснее.

Ваш разбор и улучшения

Подсказка. Здесь три грубые ошибки:

В add: условие name != null || !name.isEmpty() — если name == null, то первая часть false, а вторая — NullPointerException. Нужно &&, а не ||.
В print: условие i <= students.size() — выход за границы. Нужно i < students.size().
В getStudents: возвращает внутренний список. Снаружи можно испортить. Нужно Collections.unmodifiableList(students) или копию.

4 Раунд 4: «Парсер оценок» Средний

Код читает строку вида «Иван:4,5;Мария:5,0» и превращает её в список студентов с оценками. Здесь есть ошибки, которые выстрелят при некорректных данных.

public static void parse(String data) {\n String[] parts = data.split(";");\n for (String part : parts) {\n String[] pair = part.split(":");\n String name = pair[0];\n double grade = Double.parseDouble(pair[1]);\n System.out.println(name + " → " + grade);\n }\n }

Что нужно сделать:

Найти проблемы: непроверенные индексы, отсутствие обработки исключений, неверный формат.
Переписать: добавить проверки, обработку NumberFormatException и ArrayIndexOutOfBoundsException.
Объяснить, что произойдёт при строке «Иван» (без двоеточия) или «Иван:abc».

Ваш разбор и улучшения

Подсказка. Что произойдёт:

«Иван» без двоеточия: pair.length == 1, обращение к pair[1] бросит ArrayIndexOutOfBoundsException.
«Иван:abc»: Double.parseDouble("abc") бросит NumberFormatException.
Пустая строка: split(";") вернёт массив с пустой строкой — тоже ошибка.

Решение: проверять длину pair, оборачивать парсинг в try-catch, пропускать или логировать ошибочные записи.

5 Раунд 5: «Класс Person» Сложный

Класс Person хранит имя и возраст. Найдите все проблемы: инкапсуляция, валидация, equals, hashCode, toString, мутабельность.

public class Person {\n public String name;\n public int age;\n \n public Person(String name, int age) {\n this.name = name;\n this.age = age;\n }\n }

Что нужно сделать:

Найти проблемы: поля public, нет валидации, нет equals/hashCode, нет toString.
Переписать: сделать поля private final, добавить проверки в конструктор, реализовать методы.
Объяснить, почему это важно: что будет, если положить Person в HashSet?

Ваш разбор и улучшения

Подсказка. Проблемы:

Поля public — любой может изменить имя или возраст напрямую. Нужно private final и геттеры.
Нет валидации: можно передать null или отрицательный возраст.
Нет equals/hashCode: два Person с одинаковыми данными не равны в HashSet.
Нет toString: при печати получим что-то вроде Person@1a2b3c.

Решение: private final String name; private final int age;, проверки в конструкторе, переопределить equals, hashCode, toString. Или использовать record Person(String name, int age) — Java автоматически сделает всё это.

! Если что-то не получается важно

Код-ревью — это навык. Он приходит с практикой. Если застрял — не бросай.

            Опиши, что видишь. «Я вижу, что переменные называются a, b, c, d — и не понимаю, что в них хранится».
Опиши, что смущает. «Мне кажется, здесь можно ошибиться с индексом».
Используй подсказку. Она не даёт готовый ответ, но направляет.
Обсуди с соседом. Объяснение вслух часто помогает увидеть проблему.

        

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

★ Попробуй другие способы дополнительно

Один и тот же код можно улучшить по-разному. Вот идеи:

Раунд 1: заменить if/else if на массив порогов и цикл. Или использовать switch по диапазонам (Java 21+).
Раунд 2: вынести скидки в enum с полями percent и minSum. Или использовать паттерн «стратегия» для разных типов скидок.
Раунд 3: заменить List на Set, если студенты не должны повторяться. Или сделать класс Student с полями вместо строк.
Раунд 4: использовать регулярные выражения для проверки формата. Или Scanner с разделителями.
Раунд 5: заменить класс на record Person(String name, int age) — короче и безопаснее.

            Бонус: если найдёшь ещё один способ улучшения — напиши его в итоговом отчёте.
        

Итоговый отчёт

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

Ваш итоговый отчёт

Заполнено 0 из 6 полей