-
-
Notifications
You must be signed in to change notification settings - Fork 258
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
refactor: replace OpenStruct with Data for performance and readability improvement #3307
refactor: replace OpenStruct with Data for performance and readability improvement #3307
Conversation
Code Climate has analyzed commit 7a3ebc5 and detected 0 issues on this pull request. View more on Code Climate. |
@adrianthedev can you please review this , I think its done sorry for the delay I just got started with my masters |
@Paul-Bob would you be reviewing this ? |
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.
It feels unnatural to create a new class and override the initializer; it seems like we're not using the right tool for the job.
I came across this article discussing alternatives to OpenStruct
:
https://dev.to/lorankloeze/alternative-for-rubys-openstruct-i72
I think we should consider using a more appropriate tool, such as Struct
or Data
.
Struct
is significantly faster than OpenStruct
(~80 times faster), although I’m unsure how it compares to Data
. Let’s either research or run a quick benchmark, similar to the one in the article, to determine which option Struct
or Data
is faster, and proceed with that.
@Paul-Bob let me go through this quickly and run a quick benchmark for the same. |
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.
Thank you for working on this @Yash-Singh-Pathania
I've completed the PR by using Struct
instead of previous approaches.
Description
This PR refactors the codebase by replacing OpenStruct with Ordered Options for improved performance and immutability. The change ensures that Data is used to represent immutable objects, which provides better memory usage and execution speed compared to OpenStruct. No additional dependencies are required for this change.
Fixes # (issue)
#3245
Checklist:
Screenshots & recording
N/A (No UI changes involved)
Manual review steps
accounts
method returns the correct objects using the newData
class.perform_request
correctly handles the refactoredResponse
class in test mode.Manual reviewer: please leave a comment with output from the test if that's the case.