среда, 1 декабря 2010 г.

О декомпозиции методов

Часто приходится встречаться с длинными методами. Реакция на 'это' может сильно зависеть от сложности и времени суток: от "ничего сложного, щас разберемся!" (утром), до "дайте я кого нибудь пристрелю!" (под вечер, когда натыкаешься на 'это' под конец рабочего дня).
Попытки разбить длинный метод на 'много маленьких' не всегда заканчиваются успешно... и регулярно возникают вопросы: 'почему так?' и 'что с этим делать?'


Критерии успешности

(помимо работоспособности метода)
1. Понятность алгоритма работы.
2. Понятность алгоритма для 'непосвященного' человека.
3. Понятность алгоритма с первого, ну в крайнем случае со второго, раза.
Под словом 'алгоритм' понимается выполняемые действия.
Исходя их этих критериев, получил три правила-рекомендации:

1. Алгоритм на экран

Мэтры утверждают, что длинна метода должна быть до 5, ну максимум до 10 строк. Но в условиях наших вечно-сорванных сроков и слабо грамотных спецов, средняя длинна методов в 5 строк кажется недостижимым идеалом.
Гораздо реальным может служить размер экрана (монитора). А точнее размер области редактирования в нормальном режиме (например eclipse, меню сверху и консоль снизу).
На моем ноутбуке это примерно 25-30 строк. Очень желательно чтобы в эти 25 строк вошла сигнатура и обрамляющие скобки (дабы было видно границы метода и параметры). Поэтому получаем что то около 15-20 строк кода.

2. 'Правильное' выделение методов

Пример: есть метод. Его задача вытащить какие-то данные из БД, что то с этими данными сделать и сохранить результат.
public void doOperation(Object someCriteria){
  SQLQuery selectQuery = session.createSQLQuery("");
  selectQuery.setParameter("criteria", someCriteria);
  selectQuery.addScalar("value", Hibernate.LONG);
  List<Long> values  = selectQuery.list();
  
  long summ = 0;
  for (Long value: values){
    summ +=value;
  }
  long avg = summ / values.size();
 
  SQLQuery insertQuery = session.createSQLQuery("");
  insertQuery.setParameter("date", Calendar.getInstance());
  insertQuery.setParameter("summ", summ);
  insertQuery.setParameter("avg", avg);
}
В смысл вдумываться не надо - его там нету. Гораздо интересней подумать как не надо бы разбивать этот метод (но как часто это получается).

2.1. Метод должен отображать шаги одного 'логического уровня'

Например, в данной задаче, можно выдумать несколько уровней детализации:

Уровень 1. 'Достать данные', 'обработать данные' и 'сохранить результат'
Уровень 2. Детализация каждого шага из предыдущего уровня. Например: для 'достать данные' это a)'создать запрос', b)'подготовить запрос' и c) 'выполнить запрос'. Для 'обработать данные' это a) 'найти сумму' и b) 'вычислить среднее арифметическое'.
Уровень 3, 4, 5 и далее на сколько хватит желания и опыта в системном программировании :)

Вообщем неплохо было-бы сделать так, чтобы читатель начиная разбирать публичный метод, сразу видел шаги первого уровня, а потом уже углублялся в детали (если это нужно).
Поэтому, наверное не стоит выделять первые 3 строки, вот так:
public void doOperation(Object someCriteria){
  SQLQuery selectQuery = createSelectQuery(someCriteria);
  List<Long> values  = selectQuery.list();
  
  Results result = calculateResult(values);

  ...
Разумнее было-бы написать нечто вроде:
public void doOperation(Object someCriteria){
  List<Long> values  = extractData(someCriteria);
  Results result = calculateResult(values);
  ...

2.2. Акцент на главном

Наверное не стоит выделять только код с вычислением результата. Т.е. как то так:
public void doOperation(Object someCriteria){
  SQLQuery selectQuery = session.createSQLQuery("");
  selectQuery.setParameter("criteria", someCriteria);
  selectQuery.addScalar("value", Hibernate.LONG);
  List<Long> values  = selectQuery.list();
  
  Results result = calculateResult(values);
 
  SQLQuery insertQuery = session.createSQLQuery("");
  insertQuery.setParameter("date", Calendar.getInstance());
  insertQuery.setParameter("summ", result.getSumm());
  insertQuery.setParameter("avg", result.getAvg());
}
Получается, что мы показываем как мы лихо обращаемся с классами Hibernate, а какие-то там операции бизнес логики скромно уходит на второй план.

2.3. Имена

Ну насчет этого уже много было сказано и написано. Вообщем, как то так оно наверное повеселее было-бы:
public void doOperation(Object year){
  List<Long> monthlyIncomes= loadMonthlyIncomesForYear(year);
  Results report = prepareAnnualReport(monthlyIncomes);
  saveReport(report);
}

3. Малая глубина вложенности

В большинстве случаев интересно что делает метод и некоторые сведения как он это делает. Эту информацию можно почерпнуть из а) сигнатуры б) JavaDoc комментариев (если они есть) в) заглянув в реализацию метода.
Возмем наихудший из этих вариантов - человек глядя на сигнатуру (и не найдя комментариев) не понял что делает doOperation(Object) и начинает копаться в реализации. Очень хорошо, если при прочтении реализации doOperation(Object) все становится понятно. А если любопытство не удовлетворено? Тогда начинается изучение всех вложенных методов, а потом вложенных во вложенные и т.д.
Субъективные ощущения: если приходится изучать 'нечто' с глубиной вложенности более 2-4, то общая картина теряется.
Поэтому очень хочется, чтобы:
- либо стек вызовов был не глубже 2-3(по крайней мере в границах класса)
- либо имелись внятные сигнатуры и не-было необходимости изучать внутренности метода для понимания того, что он делает.

4. 'Обратная связь':)

wtf

2 комментария:

  1. 5-10 сток в Java вообще не достижимый идеал. т.е. очень здорово, когда получается делать такие методы. и таки часто получается, но есть масса нюансов - от заполнения Map параметрами среды исполнения REST сервиса, до инициализации объекта-холдра в 20+ полей.
    так что 5-10 строк в методе, 15-20 методов в классе, 3-5 параметров в функции - это все очень круто, равно как и свое исключение каждой исключительной ситуации. но эти правила можно и нужно нарушать в пользу читаемости, логической связности и семантической понятности кода. а еще не нужно плодить лишних сущностей - это уже старая добрая бритва Оккама
    а в остальном - все правильно сделал (С)

    ОтветитьУдалить