Страницы

Поиск по вопросам

среда, 9 января 2019 г.

Чистый код: Switch

Корректно ли выносить цепочку Switch в отдельный метод, если цепочка довольно большая? Или можно оставить ее в том же методе? Какой вариант выглядит лучше/красивее/чаще используется?


Ответ

Вы подходите не совсем правильно. Разбивать функцию на части нужно не по формальным признакам («длинный switch»), а по логическим. Вы должны задать себе вопрос: имеет ли ваш кусок кода самостоятельный смысл? (Например: можно ли сказать несколькими словами, что именно этот кусок делает?)
Если ответ на этот вопрос положительный, вынесите этот кусок в функцию, и назовите её этими самыми словами. Если отрицательный — оставляйте всё как было.

Небольшое дополнение. Кодировать switch можно по-разному. Можно оставить его как switch. Если это отображение одного объекта на другой, можно закодировать его как выборку из std::unordered_map. А возможно, ваш switch лучше представить в виде вызова виртуальной функции.
Когда именно и как правильно — снова-таки зависит от смысла. Пример:
string text; switch (code) { case 100: text = "Continue"; break; case 101: text = "Switching protocols"; break; case 200: text = "OK"; break; case 301: text = "Moved permanently"; break; case 404: text = "Not found"; break; }
по идее лучше закодировать так:
unordered_map message_text { { 100, "Continue" }, { 101, "Switching protocols" }, { 200, "OK" }, { 301, "Moved permanently" }, { 404, "Not found" } };
и в коде
string s = message_text[200];
или там
string s; auto it = message_text.find(200); if (it != message_text.end()) s = it->second;
(и выделить в отдельную функцию GetHttpMessageByCode).

Пример того, когда switch разумно заменить на иерархию классов:
switch (employee_kind) { case EmployeeKind::regular: salary = get_base_salary(); bonus = total_profit * bonus_ratio / number_of_employees; break; case EmployeeKind::external: salary = get_work_hours * get_hourly_rate(employee_id); bonus = 0; break; case EmployeeKind::manager: salary = get_base_salary(); bonus = bonus_fund / number_of_managers; if (salary < 100) bonus += 100 - salary; break; }
Такой код можно заменить на иерархию классов:
class employee { protected: int get_base_salary() { return 0; } public: virtual void compute_salary_and_bonus() = 0; };
class regular_employee : public employee { public: virtual void compute_salary_and_bonus() { salary = get_base_salary(); bonus = total_profit * bonus_ratio / number_of_employees; } };
class expernal_employee : public employee { public: virtual void compute_salary_and_bonus() { salary = get_work_hours() * get_hourly_rate(); bonus = 0; } };
class manager : public employee { public: virtual void compute_salary_and_bonus() { salary = get_base_salary(); bonus = bonus_fund / number_of_managers; if (salary < 100) bonus += 100 - salary; } };

Комментариев нет:

Отправить комментарий