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

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 returnedTree = new(testExp);
-            ExpressionTree evaluatedTree = returnedTree.Evaluate();
+            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 returnedTree = new(testExp);
-            ExpressionTree evaluatedTree = returnedTree.Evaluate();
+            ExpressionTree? evaluatedTree = returnedTree.Evaluate();
             Assert.IsTrue(evaluatedTree.Equals(exptectedTree));
         }
     }

MathEngine/MathEngine/Parser/Parser/ExpressionTree.cs

@@ -5,18 +5,18 @@ namespace MathEngine.Parser.Parser
     /// <summary>
     /// Represents an Abstract Syntax tree for expresison evaluation
     /// </summary>
-    internal class ExpressionTree
+    public class ExpressionTree
     {
         /// <summary>
         /// The root node of the expression tree;
         /// </summary>
-        private readonly TreeNode? rootNode;
+        private readonly TreeNode rootNode;
 
         /// <summary>
         /// Initialises a new instance of the MathEngine.Parser.Parser.Node class with a given Token
         /// <param name="value">The token for the nodes value</param>
         /// </summary>
-        /// <param name="Expression"></param>
+        /// <param name="Expression">String representing the </param>
         public ExpressionTree(string Expression)
         {
             List<Token> tokens = Tokeniser.Tokeniser.Tokenise(Expression);
@@ -62,53 +62,49 @@ namespace MathEngine.Parser.Parser
         /// </summary>
         /// <param name="rpnExpression">RPN expression stack to generate an expression tree from</param>
         /// <returns>An expression Tree that represents the Mathematical expression given by rpnExpression</returns>
-        private static TreeNode? GenerateExpressionTree(Stack<Token> rpnExpression)
+        private static TreeNode GenerateExpressionTree(Stack<Token> rpnExpression)
         {
             Stack<TreeNode> OutputStack = new(rpnExpression.Count);
             TreeNode Node;
             Token CurrentToken;
-            if (rpnExpression.Count == 0)
+            while (rpnExpression.Count != 0)
             {
-                return null;
+                CurrentToken = rpnExpression.Pop();
-            }
+                switch (CurrentToken.Token_Type)
-            else
-            {
-                while (rpnExpression.Count != 0)
                 {
-                    CurrentToken = rpnExpression.Pop();
+                    case Token.Type.Numeric:
-                    switch (CurrentToken.Token_Type)
+                        Node = new TreeNode(CurrentToken);
-                    {
+                        OutputStack.Push(Node);
-                        case Token.Type.Numeric:
+                        break;
-                            Node = new TreeNode(CurrentToken);
+                    // We need to preserve "Left handness" of the ExpressionTree
-                            OutputStack.Push(Node);
+                    // i.e 7/8 gives a root node of / with Cnode(0) = 7 and Cnod(1) = 8 etc.
-                            break;
+                    // This should preserve non commutativity
-                        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.Addition:  
-                        case Token.Type.Subtraction:
+                    case Token.Type.Subtraction:
-                        case Token.Type.Multiplication:
+                    case Token.Type.Multiplication:
-                        case Token.Type.Division:
+                    case Token.Type.Division:
-                        case Token.Type.Exponentiation:
+                    case Token.Type.Exponentiation:
-                            TreeNode Right = OutputStack.Pop();
+                        TreeNode Right = OutputStack.Pop();
-                            TreeNode Left = OutputStack.Pop();
+                        TreeNode Left = OutputStack.Pop();
-                            Node = CreateBinaryNode(CurrentToken, Left, Right);
+                        Node = CreateBinaryNode(CurrentToken, Left, Right);
-                            OutputStack.Push(Node);
+                        OutputStack.Push(Node);
-                            break;
+                        break;
-                        case Token.Type.UnaryPlus:
+                    case Token.Type.UnaryPlus:
-                        case Token.Type.UnaryMinus:
+                    case Token.Type.UnaryMinus:
-                            Node = CreateUnaryNode(CurrentToken, OutputStack.Pop());
+                        Node = CreateUnaryNode(CurrentToken, OutputStack.Pop());
-                            OutputStack.Push(Node);
+                        OutputStack.Push(Node);
-                            break;
+                        break;
-                    }
                 }
-                return OutputStack.Pop();
             }
+            return OutputStack.Pop();
         }
 
 
         /// <summary>
         /// Evaluates branches of a given tree
         /// </summary>
-        /// <param name="Branch"></param>
+        /// <param name="Branch">The TreeNode branch to evaluate</param>
-        /// <returns></returns>
+        /// <returns>Returns the Evaluation of the Branch</returns>
         private static TreeNode Evaluate_Tree_Branch(TreeNode Branch)
         {
             TreeNode Root, LeftBranch, RightBranch;
@@ -130,7 +126,7 @@ namespace MathEngine.Parser.Parser
         /// <param name="Operator_Token"></param>
         /// <param name="Left_Branch"></param>
         /// <param name="Right_Branch"></param>
-        /// <returns></returns>
+        /// <returns>Returns the evaluated value of the operator as a TreeNode</returns>
         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
         /// </summary>
         /// <param name="other">The object to compare to the current instance</param>
-        /// <returns></returns>
+        /// <returns>True if they are equal, False otherwise</returns>
         public override bool Equals(object? other)
         {
             if (other is TreeNode)

MathEngine/MathEngine/Parser/Parser/Node/TreeNode.cs

@@ -68,8 +68,8 @@ namespace MathEngine.Parser.Parser
         /// <summary>
         /// 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
         /// </summary>
-        /// <param name="index"></param>
+        /// <param name="index">The index of the child node to get</param>
-        /// <returns></returns>
+        /// <returns>The ChildNode at the specified index, null if it is null or the index is out-of-bounds</returns>
         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
         /// </summary>
         /// <param name="other">The object to compare to the current instance of TreeNode</param>
-        /// <returns></returns>
+        /// <returns>True if the object innstace is equal to the current ExpressionTree instance, False otherwise</returns>
         public override bool Equals(object? other)
         {
             if (other is ExpressionTree)
@@ -117,6 +117,11 @@ namespace MathEngine.Parser.Parser
             return false;
         }
 
+        /// <summary>
+        /// Returns a value that indicates if the given TreeNode instance is equal to another
+        /// </summary>
+        /// <param name="other">The TreeNode to check for equality</param>
+        /// <returns>True if they are equal, False otherwise</returns>
         public bool Equals(TreeNode other)
         {
             if (this.Value != other.Value) //If the root values are not equal we are done

MathEngine/MathEngine/Parser/Parser/Parser.cs

@@ -11,7 +11,7 @@ namespace MathEngine.Parser.Parser
         /// Return the Precdence of a given token operator
         /// </summary>
         /// <param name="X">Token to get Precdence of</param>
-        /// <returns></returns>
+        /// <returns>An integer value that represent the precedence value of the token</returns>
         private static int OperatorPrecedence(Token X)
         {
             switch (X.Token_Type)
@@ -52,7 +52,7 @@ namespace MathEngine.Parser.Parser
         /// Is the operation left associative
         /// </summary>
         /// <param name="X">Operation to check</param>
-        /// <returns></returns>
+        /// <returns>True if the token is left associative, False otherwise</returns>
         private static bool IsLeftAssociatve(Token X)
         {
             switch (X.Token_Type)
@@ -80,10 +80,10 @@ namespace MathEngine.Parser.Parser
         }
 
         /// <summary>
-        ///     ''' Reverse the order of a given stack of Tokens
+        /// Reverse the order of a given stack of Tokens
-        ///     ''' </summary>
+        /// </summary>
-        ///     ''' <param name="Stack">Stack to reverse</param>
+        /// <param name="Stack">Stack to reverse</param>
-        ///     ''' <returns></returns>
+        ///  <returns>The input stack in the reserve order</returns>
         private static Stack<Token> ReverseStackTok(Stack<Token> Stack)
         {
             Stack<Token> OutputStack = new (Stack.Count);
@@ -97,7 +97,7 @@ namespace MathEngine.Parser.Parser
         /// </summary>
         /// <param name="Expression">List of tokens to parse</param>
         /// <returns>Returns the Reverse polish notation form of the expression</returns>
-        static internal Stack<Token> Parse(List<Token> Expression)
+        internal static Stack<Token> Parse(List<Token> Expression)
         {
             //Temp holding stack for operators
             Stack<Token> OperatorStack = new(Expression.Count/2);

MathEngine/MathEngine/Parser/Tokeniser/Token.cs

@@ -3,7 +3,7 @@
     /// <summary>
     /// Defines the Token Type. The base for all manipulations
     /// </summary>
-    internal struct Token
+    internal readonly struct Token
     {
         /// <summary>
         /// Represents the token for +
@@ -137,7 +137,7 @@
         /// <summary>
         /// Debug String; Used to give a string representation of a token
         /// </summary>
-        /// <returns></returns>
+        /// <returns>Returns a string representation of the current token instance</returns>
         public new string ToString()
         {
             return Value + "," + TokenType.ToString() + "," + Numeric_Type.ToString() + "," + Arity.ToString();
@@ -145,7 +145,7 @@
 #endif
 
         /// <summary>
-        ///  Returns a value that indicates whether a two Tokens are equal.
+        ///  Returns a value that indicates whether two Tokens are equal.
         /// </summary>
         /// <param name="X">First Token to compare</param>
         /// <param name="Y">Second Token to compare</param>
@@ -172,7 +172,7 @@
         }
 
         /// <summary>
-        ///  Returns a value that indicates whether a two Tokens are not equal.
+        ///  Returns a value that indicates whether two Tokens are not equal.
         /// </summary>
         /// <param name="X">First Token to compare</param>
         /// <param name="Y">Second Token to compare</param>
@@ -198,6 +198,11 @@
             return true;
         }
 
+        /// <summary>
+        /// Returns a value that indicates whether the given Token is equal to the current Token instance.
+        /// </summary>
+        /// <param name="other">Token to compare</param>
+        /// <returns>Returns true if the two Tokens are equal and false otherwise</returns>
         public bool Equals(Token other)
         {
             if (this.TokenValue != other.TokenValue)
@@ -219,11 +224,15 @@
             return true;
         }
 
+        /// <summary>
+        /// Returns a value that indicates whether an object is equal to the current Token instance.
+        /// </summary>
+        /// <param name="obj">The object to compare to the Token instance</param>
+        /// <returns>Returns true if the object is equal to the Token instance, false otherwise</returns>
         public override bool Equals(object? obj)
         {
-            if (obj is Token)
+            if (obj is Token other)
             {
-                Token other = (Token)obj;
                 return this.Equals(other);
             }
             else

MathEngine/MathEngine/Parser/Tokeniser/Tokeniser.cs

@@ -3,7 +3,7 @@
     /// <summary>
     /// Represents the conversion of a Mathematical expression in string form to a List of Tokens
     /// </summary>
-    static internal class Tokeniser
+    internal static class Tokeniser
     {
         private static readonly List<char> 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];
-                currentIndex++;
-                curChar = currentIndex >= Expresison.Length ? '\0' : Expresison[currentIndex];
-
-            } while (char.IsWhiteSpace(curChar));
             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<Token> 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)