Skip to content

refactor: Move Context object to new class - #929

Merged
darunrs merged 17 commits into
mainfrom
refactor-context-db-into-new-class
Aug 9, 2024
Merged

darunrs merged 17 commits into
mainfrom
refactor-context-db-into-new-class

Conversation

@darunrs

@darunrs darunrs commented Jul 25, 2024 •

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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 August 8, 2024 00:30
@@ -0,0 +1,292 @@
import fetch from 'node-fetch';

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor 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
@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