Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Phone numbers #4

Closed
wants to merge 1 commit into from
Closed

Phone numbers #4

wants to merge 1 commit into from

Conversation

He4eT
Copy link

@He4eT He4eT commented Oct 6, 2019

Issue #3


const countNodes = tree => {
const branches = Object.values(tree)
const innerNodes =
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В этом случае я бы переименовал в innerNodesCount

}

const phoneNumber = numbers =>
countNodes(createTree(numbers))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Понимаю, что ты разделил логику по созданию дерева и по подсчёту узлов. Для каких-то задач – этот подход будет оптимален и удобен, но важно держать в голове насколько можно поступиться производительностью. Именно в этом случае мы проходим один раз для того, чтобы построить дерево, а второй раз считаем узлы. Кажется именно в этой задаче их можно считать один раз за первый проход, а именно в построении дерева.

/* eslint-disable-next-line */
const o1_12_123 = {1: {2: {3: {}}}}
/* eslint-disable-next-line */
const o1_12_123_124 = {1: {2: {3: {}, 4: {}}}}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 Отличная идея в названии переменной кодировать её содержимое

@@ -0,0 +1,44 @@
import test from 'ava'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Респект за тесты и за такой прокадшн подход с линтером и тестами

@evilj0e
Copy link
Owner

evilj0e commented Oct 28, 2019

Ссылку на видео пришлю чуть позже

@evilj0e evilj0e closed this Oct 28, 2019
@evilj0e
Copy link
Owner

evilj0e commented Oct 28, 2019

Спасибо за ПР. Присылай ещё 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants