-
Notifications
You must be signed in to change notification settings - Fork 230
refactor: Convert AndGate.js to AndGate.ts #917
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThe AndGate implementation was removed from its JavaScript locations and added as new TypeScript modules: deleted Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@v1/src/simulator/src/modules/AndGate.ts`:
- Around line 1-52: The class AndGate currently declares inp, inputSize, and
output1 as private which breaks external helpers like changeInputSize—make these
properties public (change private inp/inputSize/output1 to public) so
changeInputSize and other modules can access them; replace any remaining var
declarations in this module with const (e.g., loop variables) for TS
consistency; and locate any usage or declaration of the misspelled identifier
constructorParamaters and either correct it to constructorParameters or
confirm/backfill the legacy alias (keep a tolerant alias if backward
compatibility is required) so callers do not break.
- Around line 124-125: Update the v1 helplink constant for the AndGate by
replacing AndGate.prototype.helplink's old URL
'https://docs.circuitverse.org/#/chapter4/4gates?id=and-gate' with the new src
format 'https://docs.circuitverse.org/chapter4/chapter4-gates#and-gate'; apply
the same URL-format change to other gate modules (e.g.,
OrGate.prototype.helplink) so all v1 modules match the primary src helplink
pattern.
🧹 Nitpick comments (2)
src/simulator/src/modules/AndGate.ts (2)
55-59: Consider reorderingisResolvable()check before accessing input values.The current order initializes
resultfromthis.inp[0].valuebefore checkingisResolvable(). While the|| 0fallback prevents crashes, checking resolvability first is cleaner and avoids unnecessary value access when the gate cannot resolve.This appears to match the original JS implementation, so it's a minor suggestion.
♻️ Suggested reordering
resolve() { + if (this.isResolvable() === false) { + return; + } let result = this.inp[0].value || 0; - if (this.isResolvable() === false) { - return; - } for (let i = 1; i < this.inputSize; i++) {
68-68: Useconstinstead ofvarfor TypeScript consistency.For TypeScript best practices, prefer
const(orletif reassignment is needed) overvar.♻️ Proposed fix
customDraw() { - var ctx = simulationArea.context; + const ctx = simulationArea.context;
|
As stated by coderabbit, should I modify the OrGate.ts help link too? And, please tell me if I can proceed to rewrite the rest of the modules, if the current format is satisfactory. Thanks for your time :) |
|
@shoryabansalgithub please check and confirm if changes to the reference OrGate suggested by Coderabbit must be made or not, since they seem valid and reasonable to me. Can I proceed with redoing AndGate and then refactor the rest of the gates accordingly too? |
|
@Audatic07 yes you can do it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@v1/src/simulator/src/modules/AndGate.ts`:
- Around line 29-39: The logic in AndGate.ts that builds input Nodes uses raw
division (inputLength / 2) which yields floats for odd counts and produces
incorrect indices and y-positions; update the three loops that create Nodes to
use integer bounds like Math.floor(inputLength / 2) (mirroring OrGate.ts) so
indices are integers—e.g., compute half = Math.floor(inputLength/2) and use
loops with half-1, 0..half-1, and half..inputLength-1 (adjusting the y-position
expressions to use integer i and half) when constructing new Node(...) and
pushing to this.inp.
🧹 Nitpick comments (1)
v1/src/simulator/src/modules/AndGate.ts (1)
40-49: Consider aligning even-input logic with OrGate for consistency.While the even-input case produces mathematically correct node positions, the iteration pattern differs from OrGate:
- AndGate: forward iteration, nodes pushed in order
-10, -20, 10, 20- OrGate: backward then forward iteration, nodes in order
-20, -10, 10, 20This difference in
inp[]array ordering may not affect functionality but could cause subtle issues if code elsewhere relies on a specific ordering.
9ea62b1 to
b5f09ce
Compare
|
@Nihal4777 please review :) |

Addresses 1 gate migration for #908
Describe the changes you have made in this PR -
Created AndGate.ts files in v0 and v1, using OrGate.ts as reference, deleted AndGate.js files in the process.
Screenshots of the UI changes (If any) -
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
Using the provided reference of OrGate.ts, I modified AndGate.js into AndGate.ts, ensuring that I stick to the reference strictly.
Here are the changes:
Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.
Summary by CodeRabbit