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

refactor: Move Context object to new class #929

Merged
merged 17 commits into from
Aug 9, 2024

Conversation

darunrs
Copy link
Collaborator

@darunrs darunrs commented Jul 25, 2024

All logic associated with the creation of the context object resides inside the Indexer class, which bloats the file and complicates unit testing. In addition, it prevents access to the context object itself so that these calls can be made through the Indexer class itself.

This PR refactors this logic into its own class, allowing access to context methods through Indexer. The object is still recreated each time it is accessed as the object bakes in some state information (block height and log entry array) which need to be updated on subsequent accesses.

I've also reorganized the folders so that we have fewer folders under src as it is getting cluttered.

@darunrs darunrs force-pushed the refactor-context-db-into-new-class branch from 6565d01 to 5dedec3 Compare August 7, 2024 23:39
return data;
}

buildContext (blockHeight: number, logEntries: LogEntry[]): ContextObject {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about integrating the functions into this class itself, or having a persistent object to update, but I wasn't sure how that would work when frozen into the VM or with any references to class variables. I decided to continue building an object when requested.

}
}

// TODO: Migrate all below code to separate class
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All of the below code will be moved to its own class and have its own unit tests. I've simply placed them here for now to reduce PR size.

@darunrs darunrs marked this pull request as ready for review August 8, 2024 00:30
@darunrs darunrs requested a review from a team as a code owner August 8, 2024 00:30
@@ -0,0 +1,292 @@
import fetch from 'node-fetch';
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good to start nesting relating components in the filesystem to provide some additional context in to the relationships in our codebase, e.g. butting context-builder under indexer as it's directly related to that? src/ is starting to grow so would be good to start encapsulating/grouping related logic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah good point. I'll do that now then for this class and move some more in the follow up DmlHandler one.

@darunrs darunrs merged commit af08c30 into main Aug 9, 2024
3 checks passed
@darunrs darunrs deleted the refactor-context-db-into-new-class branch August 9, 2024 00:00
@darunrs darunrs linked an issue Aug 9, 2024 that may be closed by this pull request
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.

Refactor Context.Db Building to Separate Class
2 participants