Skip to content
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

corrected some typos to make the code clean and added two macros #23

Closed
wants to merge 1 commit into from

Conversation

amsorrytola
Copy link

@amsorrytola amsorrytola commented Feb 16, 2024

  1. in complex.huff the add function should take 4 values but it was written take(0) similarly it should be written return(2). It doesnt affect the working of code .
  2. added an function in complex.huff that takes a complex number and angle by which it should rotate it and returns the rotated complex number.
  3. added another function that takes two complex numbers as input and returns the equation of a line that passes through it.

@0xpanicError
Copy link
Member

Hey @mohammed-talha-ansari
Thanks for contributing and working on this repository. Can you also add some unit tests for the function you wrote to ensure that it is implemented correctly? You can refer the tests written in src/test
Thanks!

Copy link
Contributor

@blueh4mster blueh4mster left a comment

Choose a reason for hiding this comment

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

This change doesn't affect macro implementation due to the stack being the same but logically, macro definition should contain 4 instead of 0.

@0xpanicError
Copy link
Member

This change doesn't affect macro implementation due to the stack being the same but logically, macro definition should contain 4 instead of 0.

Then shouldn't it return 2 values as well? Also is this trend followed in all functions?

@blueh4mster
Copy link
Contributor

This change doesn't affect macro implementation due to the stack being the same but logically, macro definition should contain 4 instead of 0.

Then shouldn't it return 2 values as well? Also is this trend followed in all functions?

Yes, like I said what happens in the macro definition doesn't affect the actual working of the function but for clean and quality code writing, it is indeed a typo and should be takes(4) returns(2).
As for the other functions, I've checked that they don't have this typo.

@0xpanicError
Copy link
Member

This change doesn't affect macro implementation due to the stack being the same but logically, macro definition should contain 4 instead of 0.

Then shouldn't it return 2 values as well? Also is this trend followed in all functions?

Yes, like I said what happens in the macro definition doesn't affect the actual working of the function but for clean and quality code writing, it is indeed a typo and should be takes(4) returns(2). As for the other functions, I've checked that they don't have this typo.

Okay. @mohammed-talha-ansari can you also make the above change as mentioned by Preeti in this PR?

@amsorrytola amsorrytola changed the title fix some typos in functions and add some functions corrected some typos to make the code clean and added two macros Mar 6, 2024
@@ -5,7 +5,7 @@

#define constant VALUE_LOCATION = FREE_STORAGE_POINTER()

#define macro ADD_Z() = takes (0) returns (0) {
#define macro ADD_Z() = takes (4) returns (0) {
Copy link
Member

Choose a reason for hiding this comment

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

@blueh4mster can you ensure that this change is correct and doesn't introduce any bugs?

@amsorrytola amsorrytola deleted the fix-add-complex branch March 16, 2024 08:02
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.

3 participants