В этой статье на основе своей курсовой почти десятилетней давности из универа я покажу, чего при написании кода делать не стоит. Курсовая была на джаве, и я пишу на джаве, но статья может быть полезна не только джавистам, но и всем начинающим программистам. Ну и я как раз недавно прочитал Чистый Код от дяди Боба, а сейчас читаю Эффективную Джаву, так что кроме личного опыта статья будет подкреплена некоторыми идеями от больших дядь программирования.
В уже далёком 2013-м я писал курсовую (по какому предмету, уже и не помню), и основным требованием было показать знание алгоритмов и умение применять математику на практике (специальность у меня мат обеспечение информационных систем). В итоге я решил делать решатель японских кроссвордов, так как тема показалась мне не такой забитой, ну и с предметной областью я был знаком хорошо (кроссворды развлекали меня в долгих поездках на поезде, хотя сначала я увлекался больше филиппинскими – они легче были).
Описывать алгоритм решения на Джаве было очень интересно, но жаль только, что к качеству кода никаких требований не было. Код нужно было просто распечатать и приложить к работе, где описывались модель и основной алгоритм.
Теперь, спустя девять лет я решил добавить этот проект на свой гитхаб, чтобы расширить портфолио не только веб REST-проектами (ну и просто по фану). Код нужно было обновить, потому что за девять лет в джаве многое изменилось, да и мой стиль написания кода и компоновки классов поменялся очень сильно. Конечно же, я абсолютно забыл, какой алгоритм я там описал, забыл и то, как устроен мой код, поэтому теперь смотрю на него совсем свежим взглядом. И мне не нравится то, что я вижу.
Первый признак плохого кода это то, что я не могу понять, что он делает. Хотя это писал я, хотя я знаю, что там примерно должно быть, я всё равно с трудом разбираюсь, чем занимается каждый из классов и каждая из функций. Если сократить статью до одного предложения, то получится это: код должен легко читаться и пониматься – и не только вами, но и другими людьми.
А теперь разберёмся с этим в деталях. Я ни в коем случае не пытаюсь переписать упомянутые выше книги или осветить хотя бы долю тезисов оттуда. Все эти идеи соответствуют моему ужасному нечитаемому коду, поэтому и записываю я их по ходу дела и перехожу от общего к частному.
1. Разделяйте классы по пакетам для удобной навигации
Слева на изображении проект, каким он был изначально. Сейчас же я разделил все классы (да, их стало значительно больше, но об этом позже) на несколько пакетов по смыслу: отвечающие за модель данных отдельно, отвечающие за логику – отдельно, контроллеры или интерфейсы приделать мне ещё предстоит. Так легче искать файлы в дереве, не нужно пробегать глазами весь алфавитный список.
Точно не стоит сваливать всё в одну папку, как я делал 9 лет назад. Если у вас всего пара файлов и их количество расти не планирует (например, это какая-то лабораторная работа с парой функций), то можно и не напрягаться, в другом же случае лучше сразу же продумать структуру проекта.
Дерево также отражает то, насколько хорошо вы спроектировали проект (а, как известно, сначала нужно всё хорошенько обдумать и, может, даже нарисовать схемки и описать юзер-кейсы/ истории) и насколько хорошо сами понимаете происходящее. Никогда не поздно сделать рефакторинг, но чем меньше файлов вам придётся перераспределять впоследствии – тем лучше.
2. Названия должны быть говорящими.
Удачная структура помогает найти нужный класс или функцию и сориентироваться: а вообще из каких частей состоит приложение? Но в каждом пакете тоже нужно как-то ориентироваться. Чтобы это было легче, нужно использовать «говорящие» названия. То есть такие, чтобы любой человек посмотрел и понял, о чём речь. Да, даже не программист. Это как сравнивать текстовые файлы «abc 12.10.2011» и «Накладная Иванов 12.10.2011»: в первом случае даже вы можете забыть, о чём речь, а во втором даже далёкий от происходящего в вашей фирме случайный прохожий без открытия файла может догадаться, что внутри.
Поэтому не стоит использовать слишком уж общие названия, вроде foo() для функции или SomeClass для класса. То же можно сказать о переменных: text, a, b, i не особо подходящие названия для полей класса и параметров большинства функций. Для счётчиков в цикле или каких-то переменных с очень небольшой областью видимости такие названия, может, и подойдут, но в других случаях избегайте их. Если вы нечаянно где-то назвали что-то не так, то большинство современных IDE помогут вам исправить недочёт и смогут за вас найти использования этого названия в вашем проекте и переименовать её, так что не отчаиваетесь, если оставили в неудачном месте temp или опечатку.
Очень удачным примером крайней неудачного нейминга является класс с главным алгоритмом: Solvation. Начну с того, что и слова-то такого не бывает. Всё же можно догадаться о смысле: я подразумевал решение (как процесс) – от слова solve. Возможно игра слов (solve + salvation) была умышленной…
Но название класса здесь не самая большая проблема. Класс отвечает за весь алгоритм, и среди хаотично расположенных методов сложно бродить и хоть что-то извлечь.
public void makeVariations(int n, int side, int before) {
p=0;
printed=0;
this.side = side;
this.number = n;Название метода здесь хорошее, но что за n, before, p? И я бы ещё простил такой нейминг, если бы эти названия появлялись везде. Но в каждой функции свои герои: где-то это k, где-то – index, в других местах pos или number… А ведь можно было назвать всё адекватно: cursorPosition или cellInFocusIndex, если речь идёт о переборе клеток поля в поисках решения.
В общем, с правильным неймингом вы должны суметь прочитать метод и класс, почти как художественную книгу.
3. Дело должно совпадать со словом.
Мало назвать всех героев алгоритма правильно, нужно чтобы они ещё и отвечали за ваши слова. То есть если вы назвали класс Model, то он и должен лишь отвечать за репрезентацию данных. А если назвали Printer, то он должен что-то печатать. Иначе смысла нет во всех этих изощрениях с красивыми словами, если они только вводят в заблуждение.
Плжходящим примером в моём старом коде оказались классы DataJapaneseModel и JapaneseTableModel. Первый, я подумал, отвечает за данные в табличке кроссворда (судя по словам Data и Model). А второй, скорее всего, связан с графическим интерфейсом (судя по Table Model он связан с JTable). Мои сомнения бы сразу испарились, если такие классы хотя бы лежали в разных пакетах, например, с названиями Model и GUI – опять подчёркиваю важность правильной иерархии пакетов.
Но проблема в том, что эти классы совсем не такие, какими я их себе представил по названию. Посмотрите на код:
public class DataJapaneseModel extends DefaultTableModel {
private boolean Top;
private ArrayList<Integer> Numbers[] ;
private int height; //число строк
private int width; //число столбцов
private Set<TableModelListener> listeners = new HashSet<>();public class JapaneseTableModel implements TableModel {
private byte[][] Cross;
private int height; //число строк
private int width; //число столбцов
private Set<TableModelListener> listeners = new HashSet<>();Судя по двумерному массиву во втором классе, данные о всех клетках кроссворда хранятся там. Но что за числа тогда в первом классе? Список массивов (а такого вообще не бывает – не с оператором <>, во всяком случае) намекает, что там хранятся условия каждого кроссворда – наборы чисел слева и сверху от игрового поля. Но зачем тогда каждому из классов хранить ширину и высоту? И почему оба содержат сеты со «слушателями»?
Ошибка в том, что оба класса отвечают здесь и за взаимодействие с пользователем, и за хранение данных – в результате с довольно похожими названиями это создаёт лишь больше путаницы. И приводит меня к следующему совету.
4. Класс и метод должны делать как можно меньше.
В прошлом пункте я показал пару классов, которые вдвоём пытаются взять на себя слишком много. В результате между ними стирается явная разница, и разбираться в коде становится сложнее. И если вам нужно будет расширять проект, то это становится сложнее, потому что зависимости между классами не такие явные и слишком уж запутанные.
Так что убедитесь, что каждый класс отвечает за одну конкретную задачу. Если это Printer, то он печатает что-то. И всё. Кроме этого он точно ничего не считает. И окошко с выбором файла пользователю пусть тоже не открывает. Оставьте это Файл Менеджеру.
Каждый метод тоже должен делать лишь одну вещь. Если у вас в коде слишком много уровней вложенности или даже есть условный оператор, который влияет на алгоритм, это знак, что метод надо бы разбить на несколько.
Примером такой слишком загруженной функции будет search() из моего проекта:
private void search() {
if (solvation.nonfilledCells()<20) {
ArrayList<Integer[]> index = new ArrayList<>();
Integer[] temp;
for (int i=0; i<solvation.getWidth(); i++)
for (int j=0; j<solvation.getHeight(); j++)
if ((byte)solvation.getValueAt(j,i)==(byte)0) {
temp = new Integer[2];
temp[0] = j;
temp[1] = i;
index.add(temp);
}
t=false;
subgenerate(0, index, index.size());
}
else {
reserve = new JapaneseTableModel(solvation.getHeight(), solvation.getWidth());
reserve.copyCells(solvation);
for (int i=0; i<solvation.getWidth(); i++)
for (int j=0; j<solvation.getHeight(); j++)
if ((byte)solvation.getValueAt(j,i)==(byte)0) {
int temp = solved.get(solved.size()-1);
solved.clear();
solved.add(temp);
solvation.setValueAt((byte)1, j, i);
do {
for (int f=0; f<solvation.getHeight(); f++)
makeVariations(f, 0, 1);
for (int f=0; f<solvation.getWidth(); f++)
makeVariations(f, 1, 1);
}
while(!isSolved(true));
if (!checkBigSearch()) {
solvation.copyCells(reserve);
solvation.setValueAt((byte)2, j, i);
do {
for (int f=0; f<solvation.getHeight(); f++)
makeVariations(f, 0, 1);
for (int f=0; f<solvation.getWidth(); f++)
makeVariations(f, 1, 1);
}
while(!isSolved(true));
}
return;
}
}
}Во-первых, здесь нужно бы выделить блоки из if и else в отдельные методы и как-нибудь удачно их назвать. И потом их тоже можно покроить ещё на более мелкие кусочки.
5. Располагайте методы в «хронологическом» порядке.
Чтобы прошлый совет сделал код чище, нужно размещать методы в правильном порядке. Точно не стоит начинать с вспомогательных методов вроде toString() и прочих, которые почти никогда не вызываются в вашем классе. Начните с самого важного и убедитесь, что методы, которые нужны для работы этих важных и главных коллег, расположены в классе после них. Нужно сделать так, чтобы класс можно было читать сверху вниз.
6. Не налегайте на комментарии.
У меня сейчас перед глазами куча кода. И комментарии во многих случаях бы не помешали. Выше я уже показывал свои неудачные решения с неймингом. Распределение функций по классам тоже очень неудобное. В итоге мне приходится щёлкать между файлами и пытаться найти «конец» этого клубка, чтобы начать его распутывать. В общем, в этом случае мне бы не помешали комментарии, вроде «я создал хаос, но начало алгоритма в методе example()».
В случае, если код изначально написан нормально, комментарии не нужны. Названия говорят сами за себя и в комментариях не нуждаются, задача каждого класса понятна и по его содержимому и по заголовку… Зачем тут что-то комментировать?
Уж точно не надо засорять код комментариями в стиле моих записок Капитана Очевидность
private int height; //число строк
private int width; //число столбцовНу вот кому и как этот комментарий мог бы помочь? Кажется, во всём моём проекте это самое очевидное место.
Итак, в этой статье я обратил внимание на «организационные» моменты. На то, как чище организовать код по методам и классам, а их – по файлам и пакетам. Я не вдавался в подробности с тем, как правильно писать алгоритмы, и не давал конкретных советов, какие ключевые слова лучше использовать, какие типы данных стоит избегать и т. д.
Во-первых, потому что до разбора таких мелочей я ещё не добрался. Я всё ещё сижу в ужасе перед этой грудой кода и ищу в ней алгоритм. А во-вторых, сегодняшние советы могут быть актуальны и для любителей других языков.
Наверно, мне в будущем стоит сделать подборку, скажем, пятидесяти неудачных идей для джависта. Но это всё потом, а сегодня я на этом закончу и пойду переписывать алгоритм. Иногда легче написать всё заново, чем разобраться в старом коде – моя финальная полезная идея.
Zimowski Journal 