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(parse): Simplify message filtering and processing logic parseResult #476

Merged

Conversation

jeet-dhandha
Copy link
Contributor

This pull request includes changes to the parseResults function in the packages/pglite/src/parse.ts file. The main goal of these changes is to improve the readability and maintainability of the code by using a Set for valid message types and replacing conditional statements with a switch statement.

Code readability and maintainability improvements:

  • Introduced a Set named VALID_MESSAGE_TYPES to store valid message types and used it in the filter method.
  • Replaced multiple if-else statements with a switch statement to handle different message types more clearly. [1] [2] [3]

Copy link
Collaborator

@samwillis samwillis left a comment

Choose a reason for hiding this comment

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

This is a great spot, thanks @jeet-dhandha

I made a couple of small tweaks and added a changeset. Merging now 🥳

@samwillis samwillis merged commit f3f1103 into electric-sql:main Jan 13, 2025
7 checks passed
@copiltembel
Copy link
Contributor

copiltembel commented Jan 13, 2025

Nit-pick: in the worst case scenario, we're running twice over the same set: once to filter and then to process the messages. We can do both in a single run and still maintain good readability by having a default clause on the switch in which we log or gather the unsupported message types.

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