-
Notifications
You must be signed in to change notification settings - Fork 18
Code polishing checklist
Ben Parks edited this page Aug 14, 2024
·
1 revision
After completing a chunk of functionality to submit to BPCells, it's important to do a final pass through the code for style, organization, consistency, etc. This is a set of suggested concerns and questions to consider and fix as necessary before getting into a detailed final code review.
Function arguments:
- Consistency: are the names, order, and defaults consistent with similar arguments in other functions?
- Primary data argument should come first to aid use of the
|>
operator - Following arguments should come in order of most important/commonly changed
- Input/output types should work similarly to existing functions
- When applicable, inputs/outputs that can come from or be passed to other functions in the library should work without modification
- Primary data argument should come first to aid use of the
- Completeness: are all arguments used and fully handled?
- Are any arguments ignored/unused in the function body?
- For toggle-style arguments, are all options handled correctly?
- If an argument defaults to NULL, does the function handle + check for cases when it is NULL or needs to be non-NULL?
- Are there pairs/combinations of arguments where setting one causes another to be ignored? Is this clear in the documentation, or can it be avoided by adjusting the function signature?
- Robustness: Does the function check validity of arguments before all long-running calculations and return clear error messages to the user explaining problems?
- For valid options that often result in poor performance, consider a warning or API redesign
- Documentation
- Does the documentation clearly, briefly explain the purpose and valid input types/options for each argument?
- Does the intro concisely describe the purpose and scope of the function?
- Explain important details or performance considerations in a details section
- Simplicity
- Can we reduce the number of arguments without compromising important functionality?
- Is the return value the simple to understand and use for the caller?
- Can novice users get started by only paying attention to the first 1-3 arguments?
- Prefer simpler arguments (e.g. raw vector/array) over more complicated (e.g. dictionary/nested lists)
Code organization:
- Naming: Are new classes/functions/files named consistently to similar functionality?
- Placement: Is the location of the code within a file consistent with the file's existing organization? Is the location of a file consistent with the overall directory structure?
Testing:
- Functional: will the test start failing if an obvious breaking change is introduced?
- De-coupled: will the test continue to pass even if internal implementation details are changed?
- (Reasonably) comprehensive: does the test cover enough variety that we can trust it to catch accidental breakage in the future? Can we be sure a new user won't find a bug that we missed?