Несколько дней назад мой друг попросил меня помочь ему просмотреть его код. Что ж, это была моя ежедневная работа, так что я был рад помочь.
Отказ от ответственности: пример кода написан на TypeScript, однако я не знаком с этим языком. Так что поправьте меня, если я ошибаюсь.
Вот код:
Что не так с этим кодом?
Реализация этого метода длинная, давайте сначала разберем ее, чтобы мы могли поговорить о каждой части функции.
1
function getSrcTokensAndDestTokens(network)
Что такое src
dest
? Думаю, это source
и destination
соответственно. Но мы не должны этого делать, мы не должны гадать. Или мы не должны заставлять читателей угадывать, что мы хотели сказать. Итак, давайте изменим имя функции
function getSourceTokensAndDestinationTokens(...)
or
function getSourceAndDestinationTokens(...)
(Обновление: если вы чувствуете Destination
долго, мы можем использовать Target
)
Далее, network
может быть в порядке, однако это слишком распространено. Это интернет-сеть? или локальная сеть? или личная сеть? или Умбала Сеть? В этом случае мы должны дать ему более конкретное имя. Возможно gsiNetwork
.
2
const tokenInputs = this.srcTokens.concat(this.destTokens);
Да, мы должны использовать source
и destination
3
for (const token of tokenInputs) { if (network.tokens[token].hidden || network.tokens[token].delisted) { console.log(`GSI doesn't support ${token} yet!`); return; } }
Ммм… у нас есть for-loop
и if для проверки ошибок. getSrcTokensAndDestTokens
является вызывающим, мы должны упростить проверку ошибок только для этой цели. Что-то типа
if (!isValidTokenInputs) { return }
Перед рефакторингом давайте заглянем в тело цикла. network.tokens[token]
вызывается дважды. Кроме того, hidden
и delisted
не подходят для логических значений. Проведем рефакторинг
const token = network.tokens[tokenInput] // #1 if (token.isHidden || token.isDelisted) { // ... } // Note: // #1: we should use a similar name to the loop iterator, here is // `tokenInputs`, therefore, we should name `tokenInput` instead of // `token`
Следующий
console.log(`GSI doesn't support ${token} yet!`);
Этот журнал трудно найти, когда у нас есть тонны журналов. Почему? Потому что ${token}
находится посередине. Выходной журнал выглядит так:
GSI doesn't support KJHKJKY4edf5ljbjdbLLBLHJBERB yet! GSI doesn't support ljdilhKHKJBJHJGVKUYIlJNKJNBH yet! GSI doesn't support KJHJK yet!
Я также думаю, что это неправильно с точки зрения грамматики (возможно, я ошибаюсь). Мы также не должны жестко кодировать. Это должно быть
const ERROR_LOG_NOT_SUPPORTED_TOKEN = "Token not supported" // #1 // ... console.log(ERROR_LOG_NOT_SUPPORTED_TOKEN, token) // Note: // #1: We may add ERROR CODE
Хорошо, давайте сделаем for-loop
красивее
Не проще ли читать? я надеюсь, что это так
4
if (this.srcTokens.length === 0 && this.destTokens.length === 0) { console.error("srcTokens and destTokens can't be both empty!"); return; }
Ну, то же самое для жестко запрограммированного журнала ошибок. У нас должна быть константа для поиска не только внутри лог-файла, но и внутри кода. Назови это!
5
if (this.srcTokens.length === 0) { for (const token in network.tokens) { if (!network.tokens[token].hidden && !network.tokens[token].delisted) { this.srcTokens.push(token); } } } if (this.destTokens.length === 0) { for (const token in network.tokens) { if (!network.tokens[token].hidden && !network.tokens[token].delisted) { this.destTokens.push(token); } } }
Во-первых, эти два if делают одно и то же. Во-вторых, здесь у нас есть вложенный по глубине блок. Да, для меня 3 много. Наконец, можете ли вы заметить, что эти две логики одинаковы?
// #1 network.tokens[token].hidden || network.tokens[token].delisted // #2 !network.tokens[token].hidden && !network.tokens[token].delisted
Надеюсь, вы это видите. Вот правило преобразования дискретной математики
A | B = !!(A | B) = !(!A & !B)
Следуйте правилу DRY, мы должны определить метод проверки того, является ли токен действительным или нет.
function isTokenValid(token) { return !token.isHidden && !token.isDelisted }
Правило DRY также помогает объединить два оператора if в одну функцию. (Это письмо длинное, и мне лень печатать больше объяснений. Извините!)
function getValidTokens(tokens) { const result = [] for (const token in tokens) { if (isTokenValid(token) { result.push(token) } } }
И два if-s будут
const validNetworkTokens = getValidTokens(network.tokens) // #1 if (this.sourceTokens.length === 0) { this.sourceTokens = validNetworkTokens // #2 } if (this.destinationTokens.length === 0) { this.destinationTokens = validNetworkTokens // #2 } // NOTE: // #1: We can optimise by checking whether we need to create the // `validNetworkTokens` with check there is at lest a length === 0 // #2: We may need to clone the `validNetworkTokens` to avoid same // reference problem
6
Ваша задача объединить весь вышеприведенный код в окончательное решение :)
Удачного кодирования!!!