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

add homework to git, all worked, tests written #1

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Vladimir2308
Copy link
Owner

Для домашнего задания использован репозиторий
nickovchinnikov/react-js-tutorial#3
код из папки src/lesson2

Добавлена поддержка математических операторов:

возведение в квадрат **
возведение в степень ^
факториал !

Добавлена обработка скобок, в том числе вложенных
пример: (2 + 2) * 2 = 8

Добавлена обработка текста без пробелов, формулы можно вводить прямым текстом
Добавлены тесты

if (temp) {
stack.push(temp)
}
// console.log("Result: "+ stack);

Choose a reason for hiding this comment

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

54 и 56 можно убрать из коммита

Copy link
Owner Author

Choose a reason for hiding this comment

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

исправил

} else if (stack[i] === ')') {
const positionLastOpenBracket = openBracketStack.pop()
if (positionLastOpenBracket == null) {
throw new TypeError('Unexpected stack! Unexpected Bracket!')

Choose a reason for hiding this comment

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

Здесь случайно два раза вставлено, видимо

Copy link
Owner Author

Choose a reason for hiding this comment

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

текст второй части отличается от первой

Copy link

@centralToNowhere centralToNowhere left a comment

Choose a reason for hiding this comment

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

Понравилась обработка скобок, понравилось то, что можно вводить без пробелов. Единственное, npm скрипт calc не работал из за бага в ts-node. Вы наверное запускали у себя через tsc + node, но тогда лучше удалить npm скрипт и ts-node за одно. Либо в readme описать запуск.
Я бы еще добавил скрипт для линтера и husky. А так круто 👍

mathOperators.hasOwnProperty(item);
(isNumber(prevItem) &&
!isNumber(item) &&
mathOperators.hasOwnProperty(item)) ||

Choose a reason for hiding this comment

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

Eslint выдает ошибку в этом месте
les2-lint

Copy link
Owner Author

Choose a reason for hiding this comment

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

Добрый день, благодарю за уделённое время, подправил библиотеки, описал запуск в README.md, линтер пока настроил в среде, husky пока не разбирал, ошибки Eslint подправил

@@ -1,31 +1,19 @@
{
"name": "react-js-tutorial",
"name": "react-otus-homework",
"version": "1.0.0",
"description": "",
"main": "lesson2/index.js",
"scripts": {
"build": "npx webpack --mode production",
"calc": "npx ts-node src/lesson2",

Choose a reason for hiding this comment

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

npm run calc выдает ошибку Emit skipped TypeStrong/ts-node#693 (comment)
Немного смутило сначала, тесты проходят, а калькулятор не запускается! xD

Copy link
Owner Author

Choose a reason for hiding this comment

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

подправил, должно запускаться

@@ -44,7 +32,6 @@
"eslint-plugin-promise": "^5.1.0",
"eslint-plugin-react": "^7.26.1",
"html-webpack-plugin": "^4.0.3",
"husky": "^4.2.3",

Choose a reason for hiding this comment

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

а почему вы не хотите прекоммит хук с линтером? это же очень удобно, мне понравилось, когда узнал что так можно

Copy link
Owner Author

Choose a reason for hiding this comment

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

пока не успел разобрать, не всё сразу

}
const exprInBracket = listInnerBrackets.pop()
if (exprInBracket !== undefined) {
const valueInBracket = calcWithPriorities(exprInBracket)

Choose a reason for hiding this comment

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

Крутая реализация обработки скобок!

};
'**': FIRST,
'^': FIRST,
'!': FIRST,

Choose a reason for hiding this comment

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

возможно, у факториала приоритет выше, чем у возведения в степень, т е 2 ^ 3! === 2 ^ (3!), а у вас сначала степень, а потом факториал. Но это не точно, может в таких случаях без скобок не обойтись

Copy link
Owner Author

Choose a reason for hiding this comment

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

О как глубоко копнули, данного разночтения не возникает при математическом виде, а при записи всей формулы в одном регистре приоритет нигде не прописан, по мне логичным выглядит использование факториала не у степени а у числа, иначе действительно нужно использовать скобки.

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

3 participants