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

do not pass around pointers to slices and maps #174

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iuwqyir
Copy link
Collaborator

@iuwqyir iuwqyir commented Mar 13, 2025

TL;DR

Refactored BlockData handling to use value types instead of pointers for slices, improving code clarity and reducing unnecessary pointer indirection.

What changed?

This PR changes the signature of several functions across the codebase to use value types for slices instead of pointers to slices. Specifically:

  • Changed *[]common.BlockData to []common.BlockData in function parameters and return types
  • Changed *[]common.Block, *[]common.Transaction, *[]common.Log, and *[]common.Trace to their value counterparts
  • Updated all related function calls and implementations to match the new signatures
  • Simplified nil checks by using len() directly on slices instead of checking for nil pointers first
  • Updated mock implementations to reflect these changes

How to test?

  • Run the existing test suite to ensure all functionality works as expected
  • Verify that the committer and reorg handler components function correctly with the new type signatures
  • Check that storage operations (insert, get, delete) work properly with the refactored types

Why make this change?

Using pointers to slices was unnecessary and added complexity to the codebase. This change:

  1. Improves code readability by removing a layer of indirection
  2. Makes the code more idiomatic Go by using value types for slices
  3. Simplifies nil checks and length checks on collections
  4. Reduces the risk of nil pointer dereferences
  5. Makes the API more consistent and easier to understand

The change is purely refactoring and doesn't alter any business logic or functionality.

Copy link
Collaborator Author

iuwqyir commented Mar 13, 2025

@iuwqyir iuwqyir marked this pull request as ready for review March 13, 2025 19:48
@iuwqyir iuwqyir force-pushed the 03-13-do_not_pass_around_pointers_to_slices_and_maps branch from c577d09 to 89189a0 Compare March 13, 2025 19:57
@iuwqyir iuwqyir force-pushed the 03-13-do_not_pass_around_pointers_to_slices_and_maps branch 2 times, most recently from 1e5ad3e to 2d907a6 Compare March 18, 2025 10:15
@iuwqyir iuwqyir force-pushed the 03-13-do_not_pass_around_pointers_to_slices_and_maps branch from 2d907a6 to 3f090a9 Compare March 18, 2025 22:49
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