-
Notifications
You must be signed in to change notification settings - Fork 0
Postfix form entry #14
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
base: main
Are you sure you want to change the base?
Conversation
Postfixform/Postfixform/Postfix.c
Outdated
| { | ||
| push(&head, (postfixEntry[counter] - '0')); | ||
| } | ||
| else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Чтобы такой длинный else не писать, можно было continue в if сделать (только counter++ не потерять)
Postfixform/Postfixform/Postfix.c
Outdated
| return answer; | ||
| } | ||
|
|
||
| int main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше main делать в отдельном файле, иначе не получится просто скопировать файл с convertFromThePostfixForm для переиспользования в другом проекте
Postfixform/Postfixform/Postfix.c
Outdated
| #include <stdio.h> | ||
| #include <stdlib.h> | ||
|
|
||
| int convertFromThePostfixForm(char* postfixEntry) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Просили посчитать выражение, а не сконвертировать. Название функции запутывающее.
Postfixform/Postfixform/Postfix.c
Outdated
| int firstNumber = 0; | ||
| int secondNumber = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Эти штуки вне цикла не нужны, надо их объявлять в месте первого использования
Postfixform/Postfixform/Postfix.c
Outdated
|
|
||
| while (postfixEntry[counter] != '\0') | ||
| { | ||
| if (postfixEntry[counter] >= (int)('0') && postfixEntry[counter] <= (int)('9')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Совершенно незачем кастать к int. char-ы --- это обычные числа, разве что странно записывающиеся, все сравнения и арифметические операции для них тоже работают.
Postfixform/Postfixform/Postfix.c
Outdated
| } | ||
| counter++; | ||
| } | ||
| int answer = pop(&head); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если после этого стек не пуст, это тоже ошибка.
Postfixform/Postfixform/Postfix.c
Outdated
| char postfixEntry[250] = {'\0'}; | ||
| printf("enter the expression in postfix form\n"); | ||
| scanf_s("%s", postfixEntry, (unsigned)_countof(postfixEntry)); | ||
| int answer = convertFromThePostfixForm(postfixEntry); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вообще, больше const-ов этой программе бы не помешало
Stack/Stack/Stack.c
Outdated
| { | ||
| return; | ||
| } | ||
| newStack -> value = element; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| newStack -> value = element; | |
| newStack->value = element; |
Обращение к полю никогда пробелами не выделяется, хотя, казалось бы, это бинарный оператор, так что должно бы
Stack/Stack/Stack.c
Outdated
|
|
||
| int pop(Stack** head) | ||
| { | ||
| int element = (*head) -> value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если *head не NULL
Stack/Stack/Stack.c
Outdated
| { | ||
| int element = (*head) -> value; | ||
| Stack* temporary = *head; | ||
| Stack* help = (*head) -> next; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Stack* help = (*head) -> next; | |
| *head = (*head) -> next; |
Вы же уже запомнили текущую голову
yurii-litvinov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Надо ещё немного поправить
Postfixform/Postfixform/Postfix.h
Outdated
| #pragma once | ||
| #include <stdbool.h> | ||
|
|
||
| // Function that translates an expression from a postfix form |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
translates into what? :)
Stack/Stack/Stack.h
Outdated
| { | ||
| int value; | ||
| struct Stack* next; | ||
| }Stack; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }Stack; | |
| } Stack; |
Stack/Stack/Stack.h
Outdated
| struct Stack* next; | ||
| }Stack; | ||
|
|
||
| // Function for test empty stack |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Function for test empty stack | |
| // Function for testing if stack is empty |
Например. Иначе как-то не по-английски.
| Stack* temporary = *head; | ||
| *head = (*head)->next; | ||
| free(temporary); | ||
| return element; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Stack/Stack/Stack.c
Outdated
| { | ||
| if (*head != NULL) | ||
| { | ||
| int element = (*head)->value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| int element = (*head)->value; | |
| const int element = (*head)->value; |
Postfixform/Postfixform/Postfix.c
Outdated
| @@ -0,0 +1,70 @@ | |||
| #include "../../Stack/Stack/Stack.h" | |||
| #include "../../Stack/Stack/StackTest.h" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StackTest, кажется, тут не нужен. Вообще, если "боевой" код зависит от тестового, то что-то не так.
Postfixform/Postfixform/Postfix.c
Outdated
| { | ||
| if (postfixEntry[counter] >= '0' && postfixEntry[counter] <= '9') | ||
| { | ||
| push(&head, (postfixEntry[counter] - '0')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| push(&head, (postfixEntry[counter] - '0')); | |
| push(&head, postfixEntry[counter] - '0'); |
Postfixform/Postfixform/Postfix.c
Outdated
| int secondNumber = 0; | ||
| int firstNumber = 0; | ||
| secondNumber = pop(&head, ©); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не-а, переменные должны объявляться в месте первого использования
Postfixform/Postfixform/Postfix.c
Outdated
| } | ||
| int secondNumber = 0; | ||
| int firstNumber = 0; | ||
| secondNumber = pop(&head, ©); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy может быть никак не связана с check, просто bool wasError= false; pop(&head, &wasError);. Тем более что имена этих переменных не годятся
| else | ||
| { | ||
| *check = false; | ||
| return 0; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
yurii-litvinov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не пишите "fixed bugs", пишите, какие именно bugs были fixed. Ну и надо ещё чуть-чуть поправить (и всё-таки errno не использовать, это может быть не "чуть-чуть", но было же нормально).
Postfixform/Postfixform/Main.c
Outdated
| @@ -0,0 +1,36 @@ | |||
| #include "../../Stack/Stack/Stack.h" | |||
| #include "Postfix.h" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #include "Postfix.h" | |
| #include "Postfix.h" |
Postfixform/Postfixform/Main.c
Outdated
| @@ -0,0 +1,36 @@ | |||
| #include "../../Stack/Stack/Stack.h" | |||
| #include "Postfix.h" | |||
| #include "postfixFormTest.h" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
На самом деле этот файл называется с заглавной, так что под Linux программа не соберётся (там регистрозависимые файловые системы обычно, для них postfixFormTest.h и PostfixFormTest.h — это разные файлы). Поэтому обычно договариваются именовать либо всё с заглавной, либо всё со строчной.
Postfixform/Postfixform/Main.c
Outdated
| char postfixEntry[250] = { '\0' }; | ||
| printf("enter the expression in postfix form\n"); | ||
| scanf_s("%[^\n]s", postfixEntry, (unsigned)sizeof(postfixEntry)); | ||
| errno = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не стоит это использовать для возврата ошибок. Это глобальная переменная, глобальные переменные нельзя (тем более что конкретно тут можно с ума сойти проверять, что errno не перетрётся каким-нибудь вызовом стандартной библиотеки). Отдельный параметр для кода возврата ничем не хуже, разве что более громоздкий.
Postfixform/Postfixform/Main.c
Outdated
| const float answer = countTheExpression(postfixEntry); | ||
| if (errno == 1) | ||
| { | ||
| printf("Stack is empty"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я как пользователь функции ничего не знаю про стек. Тут надо более высокоуровневое сообщение об ошибке выводить, типа "у вас больше операторов, чем операндов", ну или просто что некорректное выражение.
yurii-litvinov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, теперь всё хорошо, зачтена
No description provided.