Month 12/step 1 #1
Conversation
| } | ||
| //Пока не до конца типизировано. В работе. | ||
| import { ElementStates } from '../../types/element-states'; | ||
| interface ILinkedList<T> { |
There was a problem hiding this comment.
Надо исправить: Все импорты должы располагаться в начале файла. Из-за этого могут вылазить ошибки компиляции
src/helper/helper.ts
Outdated
| Math.random().toString(36); | ||
| export function randomNumber(max: number, min: number) { | ||
| return Math.floor(Math.random() * (max - min) + min); | ||
| } |
There was a problem hiding this comment.
Можно лучше: Мне кажется этот фрагмент кода, для него лучше подходит папка utils
Обсуждение на этот счет: erikras/react-redux-universal-hot-example#808
| const [textInput, setTextInput] = useState<string>(''); | ||
| const [arrText, setArrText] = useState<TobjectText[]>([]); | ||
| const [hightIndex, setHight] = useState<number | null>(null) | ||
| const [started, setStarted] = useState<boolean>(false) |
There was a problem hiding this comment.
Можно лучше: hightIndex нигде не используется, его можно удалить
src/components/string/string.tsx
Outdated
| if(arrText.length > 1 && started == false) { | ||
| startAlgo() | ||
| } | ||
| },[arrText]) |
There was a problem hiding this comment.
Можно лучше: Мне кажется, что вместо того, чтобы проверять константу started, запускать алгоритм лучше по нажатию на кнопку (onClick)
src/components/string/string.tsx
Outdated
| }) | ||
| setHight(hight) | ||
| copy[low].style = ElementStates.Modified | ||
| copy[hight].style = ElementStates.Modified |
There was a problem hiding this comment.
Надо исправить: Сам алгоритм (технические вычисления, не рендер) стоит вынести в отдельный файл в string/utils. К примеру создать там функцию getReversingStringSteps, которая бы расписывала шаги, а рендер уже производить в компоненте
| setTimeout(() => { | ||
| res() | ||
| }, 500) | ||
| }) |
There was a problem hiding this comment.
Надо исрпавить: По аналогии стоит вынести сам алгоритм генерации чисел фибоначи в отдельный файл, а тут просто выводить результат его работы.
По аналогии в каждом компоненте алгоритов
| type="number" | ||
| isLimitText={true} | ||
| max={19} | ||
| value={numberInput} |
There was a problem hiding this comment.
Можно лучше: При числах > 19 стоит блокировать пользователю кнопку запуска
| setArr: any, | ||
| setChange: any, | ||
| setStarted: any) { | ||
| this.arr = arr; |
There was a problem hiding this comment.
Надо исправить: не стоит использовать значения any. Стоит либо их строго типизировать, либо использовать дженерики
| const [textInput, setTextInput] = useState<string>(''); | ||
| const [indexInput, setIndexInput] = useState<number>(0) | ||
| const [listArray, setListArray] = useState<any[]>([]); | ||
| const [stateChange, setChange] = useState<boolean>(false); |
There was a problem hiding this comment.
Надо исправить: по аналогии стоит типизировать хук
| <Column | ||
| key={nanoid()} | ||
| index={el.number} | ||
| state={el.style} |
There was a problem hiding this comment.
Можно лучше: При сортировке выбором стоит подсветить каким-нибудь отдельным цветом элементы которые на данным момент максимальные и минимальные
| await selectionSort(diogrammArr, setindexSort, setDiogrammArr, direction) | ||
| } | ||
| else { | ||
| await bubbleSort(diogrammArr, setindexSort, setDiogrammArr, direction) |
There was a problem hiding this comment.
Надо исправить: Сам рендер стоит оставить на компонент. Для utils только техническая часть реализации
| setterColor: React.Dispatch<React.SetStateAction<boolean>> | ||
| ) => void; | ||
| clear( | ||
| setfield:React.Dispatch<React.SetStateAction<string>> |
There was a problem hiding this comment.
Надо исправить: По аналогии с предыдущим. Рендер стоит оставить компоненту
По аналогии везде где в utils осуществляет рендер
| return ( | ||
| <li key={nanoid()} > | ||
| <Circle | ||
| head={list.head} |
There was a problem hiding this comment.
Надо исправить: если в input нет данных, то стоит заблокировать кнопки добавить
| <Circle | ||
| head={list.head} | ||
| index={index} | ||
| letter={list.text} |
There was a problem hiding this comment.
Надо исправить:
- Если удалять элементы из tail то на последнем элементе зависает, а если удалять постоянно по индексу 0, то после добавления не отображается tail
Скорее всего это связано с неверной реализацией рендера utils. Я думаю, что если организовать техническую часть в utils а рендер тут, то найти баг не составит труда
There was a problem hiding this comment.
Выглядит очень круто! Видно, что проделана большая работа.
Что важно исправить:
- Распологать в
utilsтехническую, но не сам рендер - Исправить некоторые баги в очереди
- Не использовать
anyбез необходиости - Блокировать input если в нем нет текста
Можно лучше:
- В хуке
useEffect(А также в любых функциях, что выполняют асинхронные действия) вызыватьuseStateстооит только если компонент действительно существует. Иначе, если выйти из страницы с неоконченным выполнением алгоритма возникает такая ошибка:
react_devtools_backend.js:4026 Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
at StringComponent ([http://localhost:3000/static/js/bundle.js:2603:84](http://localhost:3000/static/js/bundle.js:2603:84))
Именно поэтому важно очищать все асинхронные запросы при размонтировании компонента. Это можно сделать примерно вот так:
useEffect(() ⇒ {
let isComponentMounted = true;
await someAsyncStuff(() ⇒ {
if (isComponentMounted ) doMagic()
})
return () ⇒ {isComponentMounted = false};
})- Удалить неиспользуемые переменные
Что понравилось:
- Верстка соответствует макету и сделана хорошо
- Хорошо реализован роутинг и композиция компонентов
Обратите внимание, что работа принимается только после исправления всех «Надо исправить».
|
Поздравляю! Ваша работа принята. Вы отлично потрудились. Удачного дальнейшего обучения. |
No description provided.