Skip to content

Conversation

AlexPoziev
Copy link
Owner

No description provided.

Comment on lines +3 to +6
/// <summary>
/// Interface of the node, that performs operation element.
/// </summary>
public interface IOperandNode

Choose a reason for hiding this comment

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

Операция и операнд носят определенный семантический смысл.
Судя по описанию к интерфейсу и по тому, что класс Operation у вас реализует интерфейс IOperandNode, вы не понимаете какой :) Однако реализация решения говорит, что все-таки понимаете.
Интерфейс стоит как-то иначе назвать, чтобы не вводить читающего код в заблужление

/// Calculate divide operation.
/// </summary>
/// <returns>Result of calculation.</returns>
/// <exception cref="InvalidOperationException">SecondOperand can't be 0.</exception>

Choose a reason for hiding this comment

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

Вы же исключение другого типа выкидываете :)

Comment on lines +37 to +38
if (currentIndex > expression.Length || expression[currentIndex] != ' ')
{

Choose a reason for hiding this comment

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

Suggested change
if (currentIndex > expression.Length || expression[currentIndex] != ' ')
{
if (currentIndex >= expression.Length || expression[currentIndex] != ' ')
{

Comment on lines 108 to 109
if (!ParseTreeUtils.AreParanthesisBalanced(expression)
|| !expression.Any((x) => x == '('))

Choose a reason for hiding this comment

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

При переносе выражения на новую строку соблюдайте отступ в 8 пробелов

/// <exception cref="ArgumentException">If index out of string.</exception>
public static void IsInRange(string expression, int index)
{
if (index > expression.Length || index < 0)

Choose a reason for hiding this comment

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

Suggested change
if (index > expression.Length || index < 0)
if (index >= expression.Length || index < 0)

Choose a reason for hiding this comment

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

Не хватает тестов для случаев где в качестве expression будет передаваться null или пустая строка

@AlexPoziev AlexPoziev merged commit b5c7565 into master May 29, 2023
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