Skip to content

Postfix form entry#14

Open
MinyazevR wants to merge 38 commits intomainfrom
PostfixFormEntry
Open

Postfix form entry#14
MinyazevR wants to merge 38 commits intomainfrom
PostfixFormEntry

Conversation

@MinyazevR
Copy link
Copy Markdown
Owner

No description provided.

Comment thread Postfixform/Postfixform/Postfix.c Outdated
{
push(&head, (postfixEntry[counter] - '0'));
}
else
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Чтобы такой длинный else не писать, можно было continue в if сделать (только counter++ не потерять)

Comment thread Postfixform/Postfixform/Postfix.c Outdated
return answer;
}

int main()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Лучше main делать в отдельном файле, иначе не получится просто скопировать файл с convertFromThePostfixForm для переиспользования в другом проекте

Comment thread Postfixform/Postfixform/Postfix.c Outdated
#include <stdio.h>
#include <stdlib.h>

int convertFromThePostfixForm(char* postfixEntry)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Просили посчитать выражение, а не сконвертировать. Название функции запутывающее.

Comment thread Postfixform/Postfixform/Postfix.c Outdated
Comment on lines +11 to +12
int firstNumber = 0;
int secondNumber = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Эти штуки вне цикла не нужны, надо их объявлять в месте первого использования

Comment thread Postfixform/Postfixform/Postfix.c Outdated

while (postfixEntry[counter] != '\0')
{
if (postfixEntry[counter] >= (int)('0') && postfixEntry[counter] <= (int)('9'))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Совершенно незачем кастать к int. char-ы --- это обычные числа, разве что странно записывающиеся, все сравнения и арифметические операции для них тоже работают.

Comment thread Postfixform/Postfixform/Postfix.c Outdated
}
counter++;
}
int answer = pop(&head);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Если после этого стек не пуст, это тоже ошибка.

Comment thread 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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Вообще, больше const-ов этой программе бы не помешало

Comment thread Stack/Stack/Stack.c Outdated
{
return;
}
newStack -> value = element;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
newStack -> value = element;
newStack->value = element;

Обращение к полю никогда пробелами не выделяется, хотя, казалось бы, это бинарный оператор, так что должно бы

Comment thread Stack/Stack/Stack.c Outdated

int pop(Stack** head)
{
int element = (*head) -> value;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Если *head не NULL

Comment thread Stack/Stack/Stack.c Outdated
{
int element = (*head) -> value;
Stack* temporary = *head;
Stack* help = (*head) -> next;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
Stack* help = (*head) -> next;
*head = (*head) -> next;

Вы же уже запомнили текущую голову

Copy link
Copy Markdown

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

Надо ещё немного поправить

Comment thread Postfixform/Postfixform/Postfix.h Outdated
#pragma once
#include <stdbool.h>

// Function that translates an expression from a postfix form
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

translates into what? :)

Comment thread Stack/Stack/Stack.h Outdated
{
int value;
struct Stack* next;
}Stack;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
}Stack;
} Stack;

Comment thread Stack/Stack/Stack.h Outdated
struct Stack* next;
}Stack;

// Function for test empty stack
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
// Function for test empty stack
// Function for testing if stack is empty

Например. Иначе как-то не по-английски.

Comment thread Stack/Stack/Stack.c
Stack* temporary = *head;
*head = (*head)->next;
free(temporary);
return element;

This comment was marked as resolved.

Comment thread Stack/Stack/Stack.c Outdated
{
if (*head != NULL)
{
int element = (*head)->value;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
int element = (*head)->value;
const int element = (*head)->value;

Comment thread Postfixform/Postfixform/Postfix.c Outdated
@@ -0,0 +1,70 @@
#include "../../Stack/Stack/Stack.h"
#include "../../Stack/Stack/StackTest.h"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

StackTest, кажется, тут не нужен. Вообще, если "боевой" код зависит от тестового, то что-то не так.

Comment thread Postfixform/Postfixform/Postfix.c Outdated
{
if (postfixEntry[counter] >= '0' && postfixEntry[counter] <= '9')
{
push(&head, (postfixEntry[counter] - '0'));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
push(&head, (postfixEntry[counter] - '0'));
push(&head, postfixEntry[counter] - '0');

Comment thread Postfixform/Postfixform/Postfix.c Outdated
Comment on lines +25 to +27
int secondNumber = 0;
int firstNumber = 0;
secondNumber = pop(&head, &copy);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не-а, переменные должны объявляться в месте первого использования

Comment thread Postfixform/Postfixform/Postfix.c Outdated
}
int secondNumber = 0;
int firstNumber = 0;
secondNumber = pop(&head, &copy);
Copy link
Copy Markdown

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.

Copy link
Copy Markdown

@yurii-litvinov yurii-litvinov left a 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 не использовать, это может быть не "чуть-чуть", но было же нормально).

Comment thread Postfixform/Postfixform/Main.c Outdated
@@ -0,0 +1,36 @@
#include "../../Stack/Stack/Stack.h"
#include "Postfix.h"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
#include "Postfix.h"
#include "Postfix.h"

Comment thread Postfixform/Postfixform/Main.c Outdated
@@ -0,0 +1,36 @@
#include "../../Stack/Stack/Stack.h"
#include "Postfix.h"
#include "postfixFormTest.h"
Copy link
Copy Markdown

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 — это разные файлы). Поэтому обычно договариваются именовать либо всё с заглавной, либо всё со строчной.

Comment thread 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;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не стоит это использовать для возврата ошибок. Это глобальная переменная, глобальные переменные нельзя (тем более что конкретно тут можно с ума сойти проверять, что errno не перетрётся каким-нибудь вызовом стандартной библиотеки). Отдельный параметр для кода возврата ничем не хуже, разве что более громоздкий.

Comment thread Postfixform/Postfixform/Main.c Outdated
const float answer = countTheExpression(postfixEntry);
if (errno == 1)
{
printf("Stack is empty");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Я как пользователь функции ничего не знаю про стек. Тут надо более высокоуровневое сообщение об ошибке выводить, типа "у вас больше операторов, чем операндов", ну или просто что некорректное выражение.

Copy link
Copy Markdown

@yurii-litvinov yurii-litvinov left a comment

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.

2 participants