Распутываем сложные условия в коде
На продвинутом интенсиве по javascript мне достался на проверку случайный проект, в котором был сложный метод на 35 строк кода с четырьмя вложенными друг в друга ифами (подсвечены жёлтым):
class GameController {
/* ... */
_nextGame(position) {
state.level++;
if (state.level < this.games.length) {
if (checkAnswer(this.game.answers, this.userAnswers, this.game.type, position)) {
recordGameResult(state, getAnswerType(true, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
} else {
recordGameResult(state, getAnswerType(false, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
if (minusLives() === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
} else {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
} else {
if (checkAnswer(this.game.answers, this.userAnswers, this.game.type, position)) {
recordGameResult(state, getAnswerType(true, state.time));
} else {
recordGameResult(state, getAnswerType(false, state.time));
minusLives();
}
clearTimer(this.gameTimer);
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
}
}
}
Проект — игра, где игрок смотрит изображения и угадывает, это рисунок или фотка. В игре десять вопросов. У игрока есть три жизни. Ответил неправильно — сгорает жизнь. Если жизни кончились раньше вопросов, это проигрыш. Очки за вопрос зависят от скорости: если ответил меньше, чем за 10 секунд, получаешь больше очков. На один вопрос отводится 30 секунд, на экране тикает таймер. В конце игры результат записывается в турнирную таблицу, игрок видит свои очки и место.
В чём проблема этого кода? Он очень сложный, его тяжело понять и отладить. И в нём много дублирования. Подсвечиваю одинаковые куски одним цветом:
class GameController {
/* ... */
_nextGame(position) {
state.level++;
if (state.level < this.games.length) {
if (checkAnswer(this.game.answers, this.userAnswers, this.game.type, position)) {
recordGameResult(state, getAnswerType(true, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
} else {
recordGameResult(state, getAnswerType(false, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
if (minusLives() === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
} else {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
} else {
if (checkAnswer(this.game.answers, this.userAnswers, this.game.type, position)) {
recordGameResult(state, getAnswerType(true, state.time));
} else {
recordGameResult(state, getAnswerType(false, state.time));
minusLives();
}
clearTimer(this.gameTimer);
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
}
}
}
Смотрите. Ни одна строка не уникальна, каждое действие повторяется как минимум два раза при разных условиях. А recordGameResult (розовый), повторяется целых четыре раза. Обычно такое дублирование в разных ветках означает, что условия сгруппированы неоптимально. Можно переформулировать условия так, чтобы код не дублировался или почти не дублировался, и это сразу упрощает код.
Попробуем упростить этот код шаг за шагом. Я буду работать только с этим методом, не изменяя его окружения и не вдаваясь в подробности, как всё вокруг работает. Я буду действовать очень просто: разобью код на действия, и для каждого действия буду анализировать, при каких именно условиях это действие (и только оно) должно быть выполнено. Собственно, раскрашено уже по действиям.
Начнём с resetTimer и clearTimer. Внимательно посмотрев на код, вы убедитесь, что clearTimer будет выполнен в любом случае. Если state.level < this.games.length и если нет. Если checkAnswer вернул истину, и если он вернул ложь. clearTimer выполнится всегда. resetTimer выполнится, если state.level < this.games.length, больше выполнение resetTimer ни от чего не зависит. Но функция resetTimer просто сбрасывает состояние таймера в начальное. И не случится ничего страшного, если таймер будет сброшен и в конце игры (когда state.level сравнялся с this.game.length.
То есть функции resetTimer и clearTimer можно вызывать вообще при любых условиях. Они нужны всегда. Просто вынесем их за скобки:
class GameController {
/* ... */
_nextGame(position) {
state.level++;
resetTimer(state, initialState);
clearTimer(this.gameTimer);
if (state.level < this.games.length) {
if (checkAnswer(this.game.answers, this.userAnswers, this.game.type, position)) {
recordGameResult(state, getAnswerType(true, state.time));
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
} else {
recordGameResult(state, getAnswerType(false, state.time));
if (minusLives() === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
} else {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
} else {
if (checkAnswer(this.game.answers, this.userAnswers, this.game.type, position)) {
recordGameResult(state, getAnswerType(true, state.time));
} else {
recordGameResult(state, getAnswerType(false, state.time));
minusLives();
}
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
}
}
}
Теперь checkAnswer, выделенный голубым. Он вызывается два раза. Функция checkAnswer проверяет, верно ли ответил пользователь, и возвращает true или false. Запишу это в константу, к примеру isAnswerCorrect:
class GameController {
/* ... */
_nextGame(position) {
state.level++;
const isAnswerCorrect = checkAnswer(this.game.answers, this.userAnswers, this.game.type, position);
resetTimer(state, initialState);
clearTimer(this.gameTimer);
if (state.level < this.games.length) {
if (isAnswerCorrect) {
recordGameResult(state, getAnswerType(true, state.time));
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
} else {
recordGameResult(state, getAnswerType(false, state.time));
if (minusLives() === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
} else {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
} else {
if (isAnswerCorrect) {
recordGameResult(state, getAnswerType(true, state.time));
} else {
recordGameResult(state, getAnswerType(false, state.time));
minusLives();
}
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
}
}
}
Присмотритесь к recordGameResult, я даже уберу всё лишнее:
if (state.level < this.games.length) {
if (isAnswerCorrect) {
recordGameResult(state, getAnswerType(true, state.time));
} else {
recordGameResult(state, getAnswerType(false, state.time));
}
} else {
if (isAnswerCorrect) {
recordGameResult(state, getAnswerType(true, state.time));
} else {
recordGameResult(state, getAnswerType(false, state.time));
}
}Тут видно, что recordGameResult вызывается во всех случаях, и что единственное, что меняется, — это первый параметр у getAnswerType. Он true, когда isAnswerCorrect истина, и false, когда isAnswerCorrect ложь. Если вы хорошо усвоили предыдущий урок, то уже догадались, что для такой логики ифы не нужны, достаточно передать в getAnswerType значение isAnswerCorrect:
class GameController {
/* ... */
_nextGame(position) {
state.level++;
const isAnswerCorrect = checkAnswer(this.game.answers, this.userAnswers, this.game.type, position);
recordGameResult(state, getAnswerType(isAnswerCorrect, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
if (state.level < this.games.length) {
if (isAnswerCorrect) {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
} else {
if (minusLives() === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
} else {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
} else {
if (isAnswerCorrect) {
} else {
minusLives();
}
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
}
}
}
Теперь нетрудно заметить, что minusLives выполняется тогда и только тогда, когда isAnswerCorrect ложно. Жизни уменьшаются, если ответ неправильный. Ещё, судя по условию minusLives() === 0, эта функция возвращает оставшееся число жизней, и это число нужно запомнить. Так и сделаем, при этом самый нижний иф про isAnswerCorrect у нас исчезнет:
class GameController {
/* ... */
_nextGame(position) {
state.level++;
const isAnswerCorrect = checkAnswer(this.game.answers, this.userAnswers, this.game.type, position);
recordGameResult(state, getAnswerType(isAnswerCorrect, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
let livesLeft;
if ( ! isAnswerCorrect) {
livesLeft = minusLives();
}
if (state.level < this.games.length) {
if (isAnswerCorrect) {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
} else {
if (livesLeft === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
} else {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
} else {
// Этот иф потерял смысл, заметили? Его можно убрать.
if (isAnswerCorrect) {
} else {
}
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
}
}
}
Теперь обратим внимание на StatsModel.send (оранжевое), этот вызов сохраняет результат пользователя в турнирную таблицу. В нижнем и более простом случае это происходит тогда, когда state.level >= this.games.length (кончились уровни).
В верхнем, более сложном случае, это происходит тогда, когда
state.level < this.games.length(в игре остались непройденные уровни),- и
isAnswerCorrectложно (игрок ответил неверно), - и
livesLeft === 0(кончились жизни).
Необходимо ли выполнение этих трёх условий? Какая разница, остались ли уровни? Если кончились жизни, то кончилась игра, и надо сохранить статистику. Проверка isAnswerCorrect тоже избыточна, достаточно проверить, кончились ли жизни. Ведь жизни уже уменьшаются выше, если игрок ответил неправильно.
Статистику нужно записывать только тогда, когда игра закончилась. Игра закончилась, если кончились вопросы или жизни. Получается так:
class GameController {
/* ... */
_nextGame(position) {
state.level++;
const isAnswerCorrect = checkAnswer(this.game.answers, this.userAnswers, this.game.type, position);
recordGameResult(state, getAnswerType(isAnswerCorrect, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
let livesLeft;
if ( ! isAnswerCorrect) {
livesLeft = minusLives();
}
if (state.level >= this.games.length || livesLeft === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
}
if (state.level < this.games.length) {
if (isAnswerCorrect) {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
} else {
if (livesLeft === 0) {
} else {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
}
}
}
Остался дважды написанный жёлтый код. Его задача в том, чтобы показать следующий уровень. Поскольку это происходит в обеих ветках условия isAnswerCorrect, то от isAnswerCorrect это не зависит. А зависит только от livesLeft и условия state.level < this.games.length:
class GameController {
/* ... */
_nextGame(position) {
state.level++;
const isAnswerCorrect = checkAnswer(this.game.answers, this.userAnswers, this.game.type, position);
recordGameResult(state, getAnswerType(isAnswerCorrect, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
let livesLeft;
if ( ! isAnswerCorrect) {
livesLeft = minusLives();
}
if (state.level >= this.games.length || livesLeft === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
}
if (state.level < this.games.length) {
if (livesLeft !== 0) {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
}
}
Погодите-ка, судя по ифам (и следуя здравому смыслу), следующий уровень надо показывать тогда, когда остались жизни и уровни? Другими словами, когда игра ещё не кончилась? А предыдущий иф как раз проверяет, что она кончилась. Так значит, два ифа можно заменить на if-else:
class GameController {
/* ... */
_nextGame(position) {
state.level++;
const isAnswerCorrect = checkAnswer(this.game.answers, this.userAnswers, this.game.type, position);
recordGameResult(state, getAnswerType(isAnswerCorrect, state.time));
resetTimer(state, initialState);
clearTimer(this.gameTimer);
let livesLeft;
if ( ! isAnswerCorrect) {
livesLeft = minusLives();
}
if (state.level >= this.games.length || livesLeft === 0) {
StatsModel.send({stats: state.stats, lives: state.lives})
.then(() => app.showStats());
} else {
this.state.level = state.level;
this.game = this.games[state.level];
this.gameScreen = this._createGame(this.state, this.game);
this.screen.showScreen();
}
}
}
Готово! Дублирования в коде больше нет. Четыре вложенных ифа превратились в два невложенных. 35 значащих строк превратились в 21. И самое главное, стало понятно, какие действия в каких случаях происходят. Оказалось, случая всего два: игра продолжается или закончилась.
Когда у вас в коде несколько вложенных условий, и в них есть повторяющиеся действия, попробуйте так же, строчка за строчкой, аккуратно «распутать» ваш код. Иногда всё проще, чем кажется на первый взгляд.