<feat> Handle input expressions as binary trees instead of term:coeff dictionaries #8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the expression handling system from a term:coefficient dictionary approach to a binary tree representation. This architectural change enables more accurate comparison of complex mathematical expressions by normalizing and comparing tree structures through depth-first search (DFS) traversal.
Key Changes:
- Replaced dictionary-based expression parsing with binary tree implementation using prefix/postfix notation conversion
- Added tree normalization logic to handle commutative operations for expression equivalence checking
- Implemented comprehensive tree manipulation functions including DFS traversal and tree comparison
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
| pages/index/script.js | Complete rewrite of expression parsing and comparison logic from dictionary-based to binary tree structure, including new functions for component parsing, postfix conversion, tree building, normalization, and MathJax rendering |
| pages/index/.script.js.swp | Vim swap file accidentally included in commit (should be gitignored) |
Comments suppressed due to low confidence (1)
pages/index/script.js:288
- The indentation of this statement suggests that it is controlled by this statement, while in fact it is not.
if (type == "operator")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JoshdRod
force-pushed
the
dev-untested
branch
from
9619920 to
4485263
Compare
Added methods for converting: Valid expression -> Components Components -> Postfix Prefix (**) -> Graph
All nodes now have a property that points to their parent. This enables graph normalisation (sorting).
1. Graph normaliser takes in a binary tree, and converts it into a standard form. It does this by ensuring the content on the left node on commutative operators is lower than the right node. 2. Requires pass to convert -1 patterns into subtractions (e.g: (-1 * 10) + 5 -> 5 - 10. This prevents potential comparison errors which can occur due to this discrepancy.
…to equivalent expressions Previously, -x+f(x) would lower to a component list of -1 * x + f(x). f(x)-x would lower to f(x) - x. As a result, this expression wouldn't lower to the same tree. Now, expressionToComponentList converts subtractions to additions of -1 * the number. e.g: a-b -> a + -1 * b.
Function takes in 2 binary trees, and evaluates whether their contents are equivalent.
Function converts expression trees into MathJax expressions, which can be shown to the user.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
"Complex" in this case means "The operator's children contain one or more other operators". This means the MathJax expression will need to surround that part of the expression with brackets.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
JoshdRod
force-pushed
the
dev-untested
branch
from
2ad8dbf to
07a0c1d
Compare
This list actually stores the expression in postfix, so its name has been updated to reflect that.
'let' should be added to the "counter" variable assigned in a for loop, to ensure the variable isn't treated as global scope (can be accessed anywhere, even outside of the loop).
Quick reference for scoping in js:
- let: block scope (only exists within closest block of {}s)
- var: function scope (only exists within function)
- [none]: global scope (exists everywhere)
- const: (A constant) block scope, like let
Co-authored-by: Copilot <[email protected]>
* <feat> Add mathjax display of user input whilst typing
Errors that still need to be fixed: inputting subtractions (5-2), does not output correct result (-12+5 instead of 5-2)
Displayed expressions should not be normalised - they should be exactly the same as the user's expression!
* <fix> Fix negative numbers showing incorrectly
Negative numbers were previously showing incorrectly due to their representation in the graph as -1n.
To fix this, treeToMathJax now parses -1 nodes as '-', so the final representation is -n.
* <fix> De-normalise user MathJax expression
* <fix> Add support for quotients in operations
Previously, 1/2 + 3 failed to display correctly. This was because the \over was not being correctly wrapped around its operands.
* <fix> Fix issue with divding by x
Dividing by x generated the mathjax n\overx, which would show up as \overx, instead of /x. By adding spaces either side of the \over, we can prevent this.
Adding the left space is not needed here, but I'm keeping for good practise.
* <fix> Fix issues with quotients as function arguments
e.g: sin(x/2) would display as \sin {(x \over 2)}, where the brackets are un-needed. Removed this by replacing brackets with {} when next node is /.
* <fix><copilot> Change p from self-closing to closing tag
<p /> is not valid in html.
Co-authored-by: Copilot <[email protected]>
* <fix> Prevent invalid characters being entered into text box
* <fix> Set user input expression correctly based on character pressed
The handler functions are executed before the default event is performed. Therefore, the string comparison is done before the new key pressed is actually added to the string. e.g: If user presses backspace, the expression we have to deal with = expression in box - final character.
This patch fixes this behaviour.
* <fix> Prevent tree generation failure error by catching error and returning early
* <refactor> Add event parameter to handler function
Without passing this in as a parameter, the event object in the function refers to the non-standard global event variable - which may not work on some browsers. Hence, we pass in the actual event object.
https://teamtreehouse.com/community/why-do-we-need-the-event-object-as-parameter-for-the-callback-function
---------
Co-authored-by: Copilot <[email protected]>
Owner
Author
|
FIX:: Still having issues with commutatitivty |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 17 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously, functions used to end incorrectly. e.g: sin(x)*2 would convert to sin(x*2) in MathJax. This was due to the top operator not being popped off the operator stack during SYA.
All products of 2 expressions will now display in MathJax a conventional way.
Due to the way subtractions are handled in the expression tree (1-2 is converted to 1+(-1 * 2) when being stored, to handle commutativity(*)), extra logic is needed to convert these additions back to subtractions when displaying MathJax. * - We're talking about the commutativity of addition here. 1-2 = -2 + 1, hence the tree treats the -2 as its own little set of nodes, and considers the subtraction an addition.
… to MathJax Hard one to explain. Basically, this code needs to be refactored. But, for now, there's a problem where some expression might satisfy the outer if statement to be treated as implied *, but not the inner. In this case, since the outer if was entered, the program flow cascades through the rest of the else-ifs, and down into the + section, where it would add a +. Changing the else if's to just ifs fixes this.
To improve code readability.
Functionality was added in abb1e1c
For readability. Co-authored-by: Copilot <[email protected]>
This was referenced Dec 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Input expressions are now converted to binary tree. This has multiple ramifications: