Skip to content

Parsing tree#9

Open
Palezehvat wants to merge 14 commits intomasterfrom
ParsingTree
Open

Parsing tree#9
Palezehvat wants to merge 14 commits intomasterfrom
ParsingTree

Conversation

@Palezehvat
Copy link
Owner

4 дз

// A class that implements division
public class Divider : Operator
{
double delta = 0.0000001;

Choose a reason for hiding this comment

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

Suggested change
double delta = 0.0000001;
private const double delta = 0.0000001;

@@ -0,0 +1,26 @@
namespace ParsingTree;

// A class that implements division

Choose a reason for hiding this comment

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

Надо использовать комментарии в формате XML Documentation

double delta = 0.0000001;

// Keeps the division sign in itself
public Divider(char symbol) : base(symbol) {}

Choose a reason for hiding this comment

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

Зачем ему принимать извне division sign? Он и так знает, что он Divider

public Divider(char symbol) : base(symbol) {}

// Counts the division of two numbers on each other
public override double Calcuate(double firstValue, double secondValue)

Choose a reason for hiding this comment

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

А вот свои операнды он вполне мог бы хранить. Всё наоборот, в общем :)

Comment on lines 22 to 25
public override void Print()
{
Console.Write(" / ");
}

Choose a reason for hiding this comment

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

Тут лучше через =>

PostOrderPrint(root.Left, ref isPreviousNumber, ref sizeBackStaples);
PostOrderPrint(root.Right, ref isPreviousNumber, ref sizeBackStaples);
}
}

Choose a reason for hiding this comment

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

То же самое, это работа узлов, а не дерева. Иначе это антипаттерн "God Object"


private class Node
{
public Node()

Choose a reason for hiding this comment

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

Тут Nullability-анализ вообще недоволен, надо исправить


public class Tests
{
Tree tree;

Choose a reason for hiding this comment

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

Suggested change
Tree tree;
private Tree tree;

По традиции модификаторы доступа всегда указываются, хоть формально они и не нужны. Почему — в Java по умолчанию пакетная видимость, в C# — private, и если не указывать, будут путаться все.

Choose a reason for hiding this comment

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

И надо пустую строку

public void InTheUsualExampleTheTreeShouldCorrectlyCalculateTheValue(Tree tree)
{
tree.TreeExpression("+ 2 3");
Assert.True(tree.Calcuate() == 5);

Choose a reason for hiding this comment

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

Не, сравнение — это Assert.That(tree.Calcuate(), Is.EqualTo(5));. Иначе если тест не пройдёт, Вам напишут, что что-то false, а не то, что ожидалось 5, а вернулось то-то. Разбираться в невнятных логах непрошедших тестов — очень такое себе удовольствие, если тестов много, так что все стараются обеспечить качественные сообщения о причинах провала теста.

Comment on lines 78 to 82
private static IEnumerable<TestCaseData> TreeForTest
=> new TestCaseData[]
{
new TestCaseData(new Tree()),
};

Choose a reason for hiding this comment

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

Если тестовый случай всего один, TestCaseData не используется. Инициализируйте Tree в SetUp.

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.

3 participants

Comments