From 9c5bf4c0d50195d21ec116888eaf6df3159bc240 Mon Sep 17 00:00:00 2001 From: 0xJ1M <112640460+0xJ1M@users.noreply.github.com> Date: Thu, 11 May 2023 19:56:39 +0100 Subject: [PATCH] Feature/initial refactor (#1) * Done some initial modification of the code, removed a redundant check in GetNextChar. Updated some comments * Initial refactor: Cleaned up remaning comments --- .../Parser Tests/ExpressionTreeTests.cs | 10 +-- .../Parser/Parser/ExpressionTree.cs | 76 +++++++++---------- .../MathEngine/Parser/Parser/Node/TreeNode.cs | 11 ++- MathEngine/MathEngine/Parser/Parser/Parser.cs | 14 ++-- .../MathEngine/Parser/Tokeniser/Token.cs | 21 +++-- .../MathEngine/Parser/Tokeniser/Tokeniser.cs | 12 +-- 6 files changed, 75 insertions(+), 69 deletions(-) diff --git a/MathEngine/EngineTests/Parser Tests/ExpressionTreeTests.cs b/MathEngine/EngineTests/Parser Tests/ExpressionTreeTests.cs index 08a1862..a51bb01 100644 --- a/MathEngine/EngineTests/Parser Tests/ExpressionTreeTests.cs +++ b/MathEngine/EngineTests/Parser Tests/ExpressionTreeTests.cs @@ -23,7 +23,7 @@ namespace EngineTests TreeNode three = new(tokthree); exptectedTree.AddChildNode(four); exptectedTree.AddChildNode(three); - ExpressionTree returnedTree = new ExpressionTree(testExp); + ExpressionTree returnedTree = new(testExp); Assert.IsTrue(returnedTree.Equals(exptectedTree)); } @@ -36,8 +36,8 @@ namespace EngineTests string testExp = "3+4*7"; Token tok31 = new("31", Token.Type.Numeric, Token.NumericType.Decimal, 0); TreeNode exptectedTree = new(tok31); - ExpressionTree returnedTree = new ExpressionTree(testExp); - ExpressionTree evaluatedTree = returnedTree.Evaluate(); + ExpressionTree returnedTree = new(testExp); + ExpressionTree? evaluatedTree = returnedTree.Evaluate(); Assert.IsTrue(evaluatedTree.Equals(exptectedTree)); } @@ -51,8 +51,8 @@ namespace EngineTests decimal testValue = decimal.Divide(209 , 7); Token tok31 = new(testValue.ToString(), Token.Type.Numeric, Token.NumericType.Decimal, 0); TreeNode exptectedTree = new(tok31); - ExpressionTree returnedTree = new ExpressionTree(testExp); - ExpressionTree evaluatedTree = returnedTree.Evaluate(); + ExpressionTree returnedTree = new(testExp); + ExpressionTree? evaluatedTree = returnedTree.Evaluate(); Assert.IsTrue(evaluatedTree.Equals(exptectedTree)); } } diff --git a/MathEngine/MathEngine/Parser/Parser/ExpressionTree.cs b/MathEngine/MathEngine/Parser/Parser/ExpressionTree.cs index 9c09c54..18ba17a 100644 --- a/MathEngine/MathEngine/Parser/Parser/ExpressionTree.cs +++ b/MathEngine/MathEngine/Parser/Parser/ExpressionTree.cs @@ -5,18 +5,18 @@ namespace MathEngine.Parser.Parser /// /// Represents an Abstract Syntax tree for expresison evaluation /// - internal class ExpressionTree + public class ExpressionTree { /// /// The root node of the expression tree; /// - private readonly TreeNode? rootNode; + private readonly TreeNode rootNode; /// /// Initialises a new instance of the MathEngine.Parser.Parser.Node class with a given Token /// The token for the nodes value /// - /// + /// String representing the public ExpressionTree(string Expression) { List tokens = Tokeniser.Tokeniser.Tokenise(Expression); @@ -62,53 +62,49 @@ namespace MathEngine.Parser.Parser /// /// RPN expression stack to generate an expression tree from /// An expression Tree that represents the Mathematical expression given by rpnExpression - private static TreeNode? GenerateExpressionTree(Stack rpnExpression) + private static TreeNode GenerateExpressionTree(Stack rpnExpression) { Stack OutputStack = new(rpnExpression.Count); TreeNode Node; Token CurrentToken; - if (rpnExpression.Count == 0) + while (rpnExpression.Count != 0) { - return null; - } - else - { - while (rpnExpression.Count != 0) + CurrentToken = rpnExpression.Pop(); + switch (CurrentToken.Token_Type) { - CurrentToken = rpnExpression.Pop(); - switch (CurrentToken.Token_Type) - { - case Token.Type.Numeric: - Node = new TreeNode(CurrentToken); - OutputStack.Push(Node); - break; - case Token.Type.Addition: // We need to preserve "Left handness" i.e 7/8 gives a root node of / with Cnode(0) = 7 and Cnod(1) = 8 etc. This should preserve non commutativity - case Token.Type.Subtraction: - case Token.Type.Multiplication: - case Token.Type.Division: - case Token.Type.Exponentiation: - TreeNode Right = OutputStack.Pop(); - TreeNode Left = OutputStack.Pop(); - Node = CreateBinaryNode(CurrentToken, Left, Right); - OutputStack.Push(Node); - break; - case Token.Type.UnaryPlus: - case Token.Type.UnaryMinus: - Node = CreateUnaryNode(CurrentToken, OutputStack.Pop()); - OutputStack.Push(Node); - break; - } + case Token.Type.Numeric: + Node = new TreeNode(CurrentToken); + OutputStack.Push(Node); + break; + // We need to preserve "Left handness" of the ExpressionTree + // i.e 7/8 gives a root node of / with Cnode(0) = 7 and Cnod(1) = 8 etc. + // This should preserve non commutativity + case Token.Type.Addition: + case Token.Type.Subtraction: + case Token.Type.Multiplication: + case Token.Type.Division: + case Token.Type.Exponentiation: + TreeNode Right = OutputStack.Pop(); + TreeNode Left = OutputStack.Pop(); + Node = CreateBinaryNode(CurrentToken, Left, Right); + OutputStack.Push(Node); + break; + case Token.Type.UnaryPlus: + case Token.Type.UnaryMinus: + Node = CreateUnaryNode(CurrentToken, OutputStack.Pop()); + OutputStack.Push(Node); + break; } - return OutputStack.Pop(); - } + } + return OutputStack.Pop(); } /// /// Evaluates branches of a given tree /// - /// - /// + /// The TreeNode branch to evaluate + /// Returns the Evaluation of the Branch private static TreeNode Evaluate_Tree_Branch(TreeNode Branch) { TreeNode Root, LeftBranch, RightBranch; @@ -130,7 +126,7 @@ namespace MathEngine.Parser.Parser /// /// /// - /// + /// Returns the evaluated value of the operator as a TreeNode private static TreeNode Evaluate_Operator(Token Operator_Token, TreeNode Left_Branch, TreeNode Right_Branch) { decimal lhs = decimal.Parse(Left_Branch.NodeValue.TokenValue); @@ -161,7 +157,7 @@ namespace MathEngine.Parser.Parser if (rootNode.NodeValue.Token_Type == Token.Type.Numeric) return this; else - { + { //For now these can't be null, this will need to be updated in the future LeftBranch = Evaluate_Tree_Branch(rootNode.GetChildNode(0)); RightBranch = Evaluate_Tree_Branch(rootNode.GetChildNode(1)); Root = Evaluate_Operator(rootNode.NodeValue, LeftBranch, RightBranch); @@ -174,7 +170,7 @@ namespace MathEngine.Parser.Parser /// Returns a value indicating if the given object is equal to the current instance of ExpressionTree /// /// The object to compare to the current instance - /// + /// True if they are equal, False otherwise public override bool Equals(object? other) { if (other is TreeNode) diff --git a/MathEngine/MathEngine/Parser/Parser/Node/TreeNode.cs b/MathEngine/MathEngine/Parser/Parser/Node/TreeNode.cs index bc78b6d..da26994 100644 --- a/MathEngine/MathEngine/Parser/Parser/Node/TreeNode.cs +++ b/MathEngine/MathEngine/Parser/Parser/Node/TreeNode.cs @@ -68,8 +68,8 @@ namespace MathEngine.Parser.Parser /// /// Returns the child node specified by the index, if there are no children nodes or if the index is out of bounds than null is returned /// - /// - /// + /// The index of the child node to get + /// The ChildNode at the specified index, null if it is null or the index is out-of-bounds public TreeNode? GetChildNode(int index) { if (Children == null) @@ -107,7 +107,7 @@ namespace MathEngine.Parser.Parser /// Returns a value that indicates if the given object is equal to the current instance of TreeNode /// /// The object to compare to the current instance of TreeNode - /// + /// True if the object innstace is equal to the current ExpressionTree instance, False otherwise public override bool Equals(object? other) { if (other is ExpressionTree) @@ -117,6 +117,11 @@ namespace MathEngine.Parser.Parser return false; } + /// + /// Returns a value that indicates if the given TreeNode instance is equal to another + /// + /// The TreeNode to check for equality + /// True if they are equal, False otherwise public bool Equals(TreeNode other) { if (this.Value != other.Value) //If the root values are not equal we are done diff --git a/MathEngine/MathEngine/Parser/Parser/Parser.cs b/MathEngine/MathEngine/Parser/Parser/Parser.cs index f4b7a26..b45ae77 100644 --- a/MathEngine/MathEngine/Parser/Parser/Parser.cs +++ b/MathEngine/MathEngine/Parser/Parser/Parser.cs @@ -11,7 +11,7 @@ namespace MathEngine.Parser.Parser /// Return the Precdence of a given token operator /// /// Token to get Precdence of - /// + /// An integer value that represent the precedence value of the token private static int OperatorPrecedence(Token X) { switch (X.Token_Type) @@ -52,7 +52,7 @@ namespace MathEngine.Parser.Parser /// Is the operation left associative /// /// Operation to check - /// + /// True if the token is left associative, False otherwise private static bool IsLeftAssociatve(Token X) { switch (X.Token_Type) @@ -80,10 +80,10 @@ namespace MathEngine.Parser.Parser } /// - /// ''' Reverse the order of a given stack of Tokens - /// ''' - /// ''' Stack to reverse - /// ''' + /// Reverse the order of a given stack of Tokens + /// + /// Stack to reverse + /// The input stack in the reserve order private static Stack ReverseStackTok(Stack Stack) { Stack OutputStack = new (Stack.Count); @@ -97,7 +97,7 @@ namespace MathEngine.Parser.Parser /// /// List of tokens to parse /// Returns the Reverse polish notation form of the expression - static internal Stack Parse(List Expression) + internal static Stack Parse(List Expression) { //Temp holding stack for operators Stack OperatorStack = new(Expression.Count/2); diff --git a/MathEngine/MathEngine/Parser/Tokeniser/Token.cs b/MathEngine/MathEngine/Parser/Tokeniser/Token.cs index 52f3ce3..0e05502 100644 --- a/MathEngine/MathEngine/Parser/Tokeniser/Token.cs +++ b/MathEngine/MathEngine/Parser/Tokeniser/Token.cs @@ -3,7 +3,7 @@ /// /// Defines the Token Type. The base for all manipulations /// - internal struct Token + internal readonly struct Token { /// /// Represents the token for + @@ -137,7 +137,7 @@ /// /// Debug String; Used to give a string representation of a token /// - /// + /// Returns a string representation of the current token instance public new string ToString() { return Value + "," + TokenType.ToString() + "," + Numeric_Type.ToString() + "," + Arity.ToString(); @@ -145,7 +145,7 @@ #endif /// - /// Returns a value that indicates whether a two Tokens are equal. + /// Returns a value that indicates whether two Tokens are equal. /// /// First Token to compare /// Second Token to compare @@ -172,7 +172,7 @@ } /// - /// Returns a value that indicates whether a two Tokens are not equal. + /// Returns a value that indicates whether two Tokens are not equal. /// /// First Token to compare /// Second Token to compare @@ -198,6 +198,11 @@ return true; } + /// + /// Returns a value that indicates whether the given Token is equal to the current Token instance. + /// + /// Token to compare + /// Returns true if the two Tokens are equal and false otherwise public bool Equals(Token other) { if (this.TokenValue != other.TokenValue) @@ -219,11 +224,15 @@ return true; } + /// + /// Returns a value that indicates whether an object is equal to the current Token instance. + /// + /// The object to compare to the Token instance + /// Returns true if the object is equal to the Token instance, false otherwise public override bool Equals(object? obj) { - if (obj is Token) + if (obj is Token other) { - Token other = (Token)obj; return this.Equals(other); } else diff --git a/MathEngine/MathEngine/Parser/Tokeniser/Tokeniser.cs b/MathEngine/MathEngine/Parser/Tokeniser/Tokeniser.cs index 46de9f2..827c30c 100644 --- a/MathEngine/MathEngine/Parser/Tokeniser/Tokeniser.cs +++ b/MathEngine/MathEngine/Parser/Tokeniser/Tokeniser.cs @@ -3,7 +3,7 @@ /// /// Represents the conversion of a Mathematical expression in string form to a List of Tokens /// - static internal class Tokeniser + internal static class Tokeniser { private static readonly List Operators = new(new char[] { '+', '-', '*', '/', '^' }); @@ -13,12 +13,8 @@ static private char GetNextChar(string Expresison, ref Int32 currentIndex) { char curChar; - do - { - currentIndex++; - curChar = currentIndex >= Expresison.Length ? '\0' : Expresison[currentIndex]; - - } while (char.IsWhiteSpace(curChar)); + currentIndex++; + curChar = currentIndex >= Expresison.Length ? '\0' : Expresison[currentIndex]; return curChar; } @@ -55,7 +51,7 @@ //Cleanup whitespace Expression = String.Concat(Expression.Where(c => !Char.IsWhiteSpace(c))); Int32 currentIndex = -1; - //Example expression 1+1+Sin[x] + List Tokenstack = new(Expression.Length); //Nearly always is overallocated to the true number of tokens but avoids the need to kkeep reallocating for a growing stack char curChar; while (currentIndex < Expression.Length)