Skip to content

<refactor> Refactor Tree to standardised object#29

Merged
JoshdRod merged 27 commits intomainfrom
refactor-tree
Feb 2, 2026
Merged

<refactor> Refactor Tree to standardised object#29
JoshdRod merged 27 commits intomainfrom
refactor-tree

Conversation

@JoshdRod
Copy link
Copy Markdown
Owner

Tree object contains the following:
DATA:
-- list body, contains node objects making up the tree
-- int root, pointer to root node in tree
-- int length, length of body
FUNCTIONALITY:
-- Get(index) - gets node at specified index
-- Find(Node) - finds index of node in tree
-- Add(Node, Node parentNode) - adds node to the tree under parentNode
-- Remove(Node) - removes node from the tree

The key feature of this change is that nodes can be removed from the tree without ruining the pointers.
Previously, removing a node from the tree would mean that all pointers that pointed to an index greater than the element removed would be off by 1. The Remove(Node) funciton handles modifying the pointers in the tree, so this is no longer an impossibility.

Node class contains properties:
- type
- content
- leftNode
- rightNode
- parent
- commutative
- precedence
Previously, expressionToComponentList wasn't able to use the new Node class, for a few reasons:
- The Node class needs to be defined before it is accessed (see the Temporal Dead Zone https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#temporal_dead_zone_tdz)
- Move Node class to a new file
- Add private type and content properties, which are accessed by the Node getter and setters
- Change node type references to the enum, instead of string
- Fix spelling errors
- Change references to node type to reference the enum
- Change references to node type to reference the enum
- Replace references to string literals in the function to use the NodeType and Operator enums
Returning in a switch-case statement terminates the statement. Hence, a break after the return is just dead code, and unneeded.
JoshdRod and others added 4 commits February 1, 2026 13:52
Previously, a missing break caused the setting of content for any Operator Node to fall into the function case.
e.g: This would set its content as '+', instead of OPERATOR.ADDITION.
Currently, these two things are the same thing, but this could deviate in the future.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Error objects return their stack trace, unlike a raw string.
Base automatically changed from refactor-node to main February 1, 2026 14:28
Tree object contains the following:
DATA:
-- list body, contains node objects making up the tree
-- int root, pointer to root node in tree
FUNCTIONALITY:
-- Add(Node, Node parentNode) - adds node to the tree under parentNode
-- Remove(Node) - removes node from the tree

Constructor and functionality still needs to be implemented
Add function now adds nodes to tree
Still need to test Add and Remove function - after I've done constructor, I should be able to
Tree object can be instantiated with a body (list of node objects)
- Syntax errors fixed
- Renamed parentNode to parent
- The  keyword isn't needed for class methods. The class has been fixed to reflect this.
Make it possible to retrieve the Nodes inside the tree, by calling the Get function.
Unfortunately, JS doesn't support function overloading. Hence, we have to settle here for a helper method.
Beginning of migrating all functions over to the Tree object
Functions in script.js now use the Tree object when interacting with a tree.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the expression “tree” representation into standardized Node/Tree classes and updates the existing parsing/normalisation code to use the new API, with the goal of enabling safe node removal without breaking index-based pointers.

Changes:

  • Added pages/index/tree.js introducing Operator, NodeType, Node, and Tree abstractions.
  • Updated pages/index/script.js to construct Node instances, use NodeType/Operator constants, and interact with trees via Tree.Get() / Tree.Find().
  • Updated index.html to load tree.js before script.js.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.

File Description
pages/index/tree.js Introduces the new standardized Tree/Node object model and pointer-management logic (incl. removal).
pages/index/script.js Migrates parsing and tree algorithms from plain objects/arrays to the new Node and Tree APIs.
index.html Ensures the new tree module is loaded for browser runtime before dependent code.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pages/index/tree.js
Comment thread pages/index/script.js Outdated
Comment thread pages/index/tree.js Outdated
Comment thread pages/index/tree.js Outdated
Comment thread pages/index/tree.js Outdated
Comment thread pages/index/tree.js
Comment thread pages/index/tree.js
Comment thread pages/index/script.js
Comment thread pages/index/tree.js
Comment thread pages/index/tree.js Outdated
In Add(), the parent did not have to be in the tree to be added to. This is now not possible.
In Remove(), the node removed did not have to be in the tree. This is now also not possible.

This prevents unintended consequences of phantom trees being created / phantom nodes messing up pointers.
@JoshdRod
Copy link
Copy Markdown
Owner Author

JoshdRod commented Feb 1, 2026

TODO:

  • Switch errors to throw Error objs
  • Put up issue for these types of errors:
image

JoshdRod and others added 2 commits February 2, 2026 18:19
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Error objects contain the stack trace of the program - throwing a string doesn't.
Previously, eTCL used to set these properties on the component objects it created. Now, the Node object computes these values at request. Hence, these locals are dead code and can be removed.
If root index > index of node being removed, we need to -1 from tree.root to ensure it still points towards the correct index.
@JoshdRod JoshdRod merged commit 217882c into main Feb 2, 2026
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