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

Month 12/step 1 #1

Merged
merged 32 commits into from
Jun 5, 2022
Merged

Month 12/step 1 #1

merged 32 commits into from
Jun 5, 2022

Conversation

c1ty-cat
Copy link
Owner

No description provided.

import { DELAY_IN_MS, SHORT_DELAY_IN_MS } from "../constants/delays"

export const waitForMe = (delay: number = DELAY_IN_MS): Promise<null> => {
return new Promise(resolve => {

Choose a reason for hiding this comment

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

Можно лучше: название я оценил :D, но все же лушче дать более конкретное и понятное имя вроде delay


export const swapNums = (
arr: columnObject[],
firstIndex: number,

Choose a reason for hiding this comment

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

Можно лучше: функции swapNums и swapChars практически идентичны. Стоит обьеденить их в 1

/>
<Button
disabled={inputValue ? inputValue > 19 : true}
isLoader={inProgress}

Choose a reason for hiding this comment

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

Можно лучше: магические число 19 лучше вынести в отдельную константу

const arr: columnObject[] = [];
const size = Math.random() * (17 - 3) + 3;
while (arr.length < size) {
arr.push({

Choose a reason for hiding this comment

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

Можно этот кусок кода записать проще:

const rSize = Math.random() * (17 - 3) + 3;
Array.from({length: rSize}, () => ({num: utils.getHeight(), state: ElementStates.Default}))

};

const bubbleSort = async (mode: "ascending" | "descending") => {
// Лочим кнопки

Choose a reason for hiding this comment

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

Надо исправить: Сами функции (как и во всех остальных компонентах), которые отвечают за сам алгоритм нужно вынести в utils.tsx в папке этого компонента


const push = async () => {
setInputValue("");
setPushing(true);
Copy link

@kyzinatra kyzinatra May 29, 2022

Choose a reason for hiding this comment

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

Надо исправить: В этом случае можно создать класс Stack где реализовать эти методы и сюда уже их импротить

/>
<Button
style={{ minWidth: "362px" }}
extraClass={styles.button}

Choose a reason for hiding this comment

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

Можно лучше: не стоит использовать in-line стили, лучше вынести их в класс

Copy link

@kyzinatra kyzinatra left a comment

Choose a reason for hiding this comment

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

Выглядит очень круто! Видно, что проделана большая работа.

Что важно исправить:

  • В файле readme не нашел описание проекта
  • функции, которые реализуют работу алгоритма должны быть вынесены в отдельный utils.tsx в папке соответствующего компонента. В utils.tsx стоит считать шаги алгоритма (если сам результат уже не описывает шаги, как в случае последовательности фибоначи) и далее рендерить их в react
  • Стоит создать класс Queue для реализации очереди и класс LinkedListNode и ласс LinkedList с методами prepend, append, addByIndex, deleteByIndex, deleteHead, deleteTailи toArray

Можно лучше:

  • файл .editorconfig стоит добавить в .gitignore так как он локальный для вас и не используется другими
  • Для удобства можно установить "noImplicitAny": true в ts config

Что понравилось:

  • Отличное комментирование кода

Обратите внимание, что работа принимается только после исправления всех «Надо исправить».

@kyzinatra
Copy link

kyzinatra commented May 31, 2022

Вы отлично потрудились, но пункт чеклиста состоит немного не в этом. Видно, что вы старались вынося это в отдельные файлы, и если честно работа меня впечатлила, но вы верно сделали в 1 раз, что организовывали рендеринг в самом компоненте. Это правильно, но вы таже в самом компоненте организовали и вычисление самого алгоритма. Давайте я покажу на примере как это может выглядеть:
К примеру fibonacci-page

В самом файле utils.ts я реализую только алгоритм.

export function getFibonacciNumbers(count: number) {
  const fibonacciNumbers = [1, 1];
  for (let i = 2; i <= count; i++) {
    const a = fibonacciNumbers[i - 1];
    const b = fibonacciNumbers[i - 2];
    fibonacciNumbers.push(a + b);
  }
  return fibonacciNumbers;
}

Как видно очень простая функция

А теперь в самом компоненте я провожу рендер этих данных:
(Привожу в качестве примера, вы можете использовать это по своему разумению)

// fibonacci-page.tsx

const [numberCount, setNumberCount] = useState(0);
  const fibonacciNumbers = useRef<number[]>([]);
  const intervalId = useRef<NodeJS.Timeout>();


 const startAlgorithm = () => {
    fibonacciNumbers.current = getFibonacciNumbers(numberCount);
    setCurrentAlgorithmStep(0);

    intervalId.current = setInterval(() => {
      if (numberCount > 0) {
        setCurrentAlgorithmStep((currentStep) => {
          const nextStep = currentStep + 1;

          if (nextStep > numberCount && intervalId.current) {
            clearInterval(intervalId.current);
          }

          return nextStep;
        });
      }
    }, SHORT_DELAY_IN_MS);
  };

...

return (
... 
 fibonacciNumbers.current
              .slice(0, currentAlgorithmStep)
              .map((fibonacciNumber, index) => { ... } ) 
...
)

Или к примеру list-page:

В linked-list.ts, utils.ts по чеклисту стоит вынести только лишь реализацию списка, а рендер, конечно, производится в компоненте
(Рендер-часть можно удобнее организовать с помощью пользовательских хуков, но это уже более сложная тема и спринт этого не требует)

// utils.ts  пример реализации одной функции

export function addTail<T>(value: T, list: LinkedList<T>): Step<T>[] {
  const steps: Step<T>[] = [];
  const sourceListValues = list.toArray();

  steps.push({
    index: sourceListValues.length - 1,
    value,
    listValues: sourceListValues,
  });

  list.append(value);

  steps.push({
    index: sourceListValues.length,
    listValues: list.toArray(),
  });

  steps.push({
    listValues: list.toArray(),
  });

  return steps;
}
// linked-list.ts
export class LinkedListNode<T> {
  ...
}

export class LinkedList<T> {
...
}

Посмотрите внимательно чеклист по соответствующим категориям:
image

image

По аналогии по каждой категории для которой это актуально

@kyzinatra
Copy link

kyzinatra commented Jun 4, 2022

Отличная работа! Желаю успехов в дальнейшем развитии и обучении

@c1ty-cat c1ty-cat merged commit 441e8c6 into master Jun 5, 2022
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.

2 participants