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

Parse Tree #7

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

Parse Tree #7

wants to merge 7 commits into from

Conversation

artemiipatov
Copy link
Owner

No description provided.

@@ -0,0 +1,15 @@
namespace ParseTree;

public interface INode

Choose a reason for hiding this comment

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

Надо комментарий и к самому интерфейсу, и это даже важнее, чем комментарии к его методам

/// <summary>
/// Evaluates value of current node subtree.
/// </summary>
/// <returns></returns>

Choose a reason for hiding this comment

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

А вот пустые тэги не надо

Comment on lines 5 to 9
enum NodeType
{
Operator,
Operand
}

Choose a reason for hiding this comment

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

Антипаттерн "Тэг типа" :) По идее, дереву должно быть всё равно, оператор это или операнд

/// </summary>
/// <param name="value"></param>
/// <param name="operandOrOperator"></param>
void AddNode(int value, NodeType operandOrOperator);

Choose a reason for hiding this comment

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

У оператора не может быть value

/// <summary>
/// Parent node of the current node.
/// </summary>
public INode? Parent { get; set; }

Choose a reason for hiding this comment

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

И у оператора, и у операнда есть это свойство, можно поднять его в их общего предка

{
if (_currentNode == null)
{
throw new Exception("Wrong input");

Choose a reason for hiding this comment

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

Вообще, просто Exception кидать нельзя

{
throw new Exception("Wrong input");
}
Operand newNode = new Operand(value) { Parent = _currentNode };

Choose a reason for hiding this comment

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

Suggested change
Operand newNode = new Operand(value) { Parent = _currentNode };
var newNode = new Operand(value) { Parent = _currentNode };

testTree.Parse("../../../testExpression.txt");
Assert.AreEqual(testTree.Eval(), 957);

strWriter = new StreamWriter(File.Open("../../../testExpression.txt", FileMode.Create));

Choose a reason for hiding this comment

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

Несколько тестовых сценариев --> несколько тестов

[Test]
public void EvaluationOfLargeExpressionsShouldBeCorrect()
{
StreamWriter strWriter = new StreamWriter(File.Open("../../../testExpression.txt", FileMode.Create));

Choose a reason for hiding this comment

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

Suggested change
StreamWriter strWriter = new StreamWriter(File.Open("../../../testExpression.txt", FileMode.Create));
var strWriter = new StreamWriter(File.Open("../../../testExpression.txt", FileMode.Create));

strWriter.Close();
IParseTree testTree = new ParseTree();
testTree.Parse("../../../testExpression.txt");
Assert.AreEqual(testTree.Eval(), 957);

Choose a reason for hiding this comment

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

Наоборот, Assert.AreEqual(957, testTree.Eval());

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