Refactor block_cemented_callback for readability #4306
Conversation
- Modularized the block confirmation logic into separate functions. - Separated active and inactive block confirmation processes. - Simplified the main block_cemented_callback() function for easier understanding. - Reduced nested conditionals by early returning and using helper methods. - Improved code readability and maintainability.
nano/node/active_transactions.cpp
Outdated
| } | ||
| } | ||
|
|
||
| void nano::active_transactions::activate_scheduler_for_account_and_destination (const nano::account & account, std::shared_ptr<nano::block> const & block, nano::store::read_transaction const & transaction) |
There was a problem hiding this comment.
This name is a bit long. Maybe "activate_dependents".
There was a problem hiding this comment.
I agree that it's too long. Since it activates the next blocks, what about "activate_successors" ?
- auto transaction
- switch statement
- rename to `election_status`
- remove redundant {}
- use initialization syntax amount{ 0 }
- rename to `handle_confirmation`
- rename to `activate_successors`
|
Thanks for the feedback. I applied all your suggestions. |
I've been moving away from that convention. Since argument names are looked at by the users I think it's better to keep those clean. I still use _m (class member) and _l (local variable) when there are name shadowing issues because those aren't often looked at by end users. |
| process_inactive_confirmation (transaction, block_a); | ||
| break; | ||
|
|
||
| default: |
There was a problem hiding this comment.
I don't think we should use a default clause here but that's the existing functionality so I guess we can keep it.
Marked as draft PR to discuss if such a breakup into smaller methods is desirable