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

Small code refactor #3363

Open
wants to merge 7 commits into
base: mainnet-staging
Choose a base branch
from
Open

Conversation

ylmin
Copy link

@ylmin ylmin commented Jul 13, 2024

Motivation

  • Abstract the logic for processing messages: Abstract the logic for signing and verification into the process_message function to reduce duplicate code.
  • Abstract network execution logic: Abstract the network-related execution logic into the execute_for_network function to reduce duplicate code.
  • Maintain single responsibility: Although some functions are merged, each function still maintains a single responsibility to ensure code readability and maintainability.
  • Merge account generation logic: The generate_account function is further refined into new_vanity_account and new_seeded_account.

Test Plan

(If you changed any code, please provide clear instructions on how you verified your changes work.)

Related PRs

(Link any related PRs here)

Error Handling Improvement: In the `enable_listener` method, use if let Err(e) for error handling instead of directly using expect. This can prevent the program from crashing when an error occurs. The expect method will cause the program to crash and print the specified error message when an error is encountered. This is useful during development and debugging because it helps quickly locate the problem. However, in a production environment, directly crashing may not be the best choice as it can lead to service interruptions.
@vicsn
Copy link
Contributor

vicsn commented Jul 15, 2024

While more DRY, this also makes the code more complex and tightly coupled. I suggest instead taking a look at existing issues to see what the community is interested in

@ylmin
Copy link
Author

ylmin commented Jul 15, 2024

While more DRY, this also makes the code more complex and tightly coupled. I suggest instead taking a look at existing issues to see what the community is interested in

Maintain single responsibility: Although some functions are merged, each function still maintains a single responsibility to ensure code readability and maintainability.

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.

2 participants