Notice: this Wiki will be going read only early in 2024 and edits will no longer be possible. Please see: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/wikis/Wiki-shutdown-plan for the plan.
Difference between revisions of "OSEE/Software Development Process/Peer Review Checklist"
(→Classes) |
|||
(One intermediate revision by the same user not shown) | |||
Line 1: | Line 1: | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
==Arithmetic== | ==Arithmetic== | ||
# Are divisors tested for zero? | # Are divisors tested for zero? | ||
# Do comparisons of floating point numbers have tolerance? | # Do comparisons of floating point numbers have tolerance? | ||
− | == | + | ==Change Management== |
− | # | + | # Does the commit have a single focus? |
− | + | # Is the commit too complicated to review effectively? | |
− | + | ||
− | + | ||
− | # | + | |
− | + | ||
− | + | ||
− | + | ||
==Classes== | ==Classes== | ||
Line 39: | Line 15: | ||
# Does the class have a clear lifecycle? | # Does the class have a clear lifecycle? | ||
− | == | + | ==Code Style/Formatting== |
− | + | # Has the standard formatter been applied? | |
− | + | # Are methods and classes within size guidelines? | |
− | + | # Are coding guidelines checked with tools? | |
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | # | + | |
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | # Are | + | |
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | + | ||
− | # | + | |
==Comments== | ==Comments== | ||
Line 75: | Line 27: | ||
# Are comments only used for technical notes? | # Are comments only used for technical notes? | ||
# Are any code segments commented out? | # Are any code segments commented out? | ||
+ | |||
+ | ==Concurrency== | ||
+ | # Are there mutable member variables in classes with static methods that can be called concurrently? | ||
+ | |||
+ | ==Configuration== | ||
+ | # Is the constant or configurable item acquired by a centralized object? | ||
+ | # Does the item need to be configurable? | ||
==Control Flow== | ==Control Flow== | ||
Line 94: | Line 53: | ||
# Are runtime and checked exceptions and throwable java.lang.error considered and handled appropriately? | # Are runtime and checked exceptions and throwable java.lang.error considered and handled appropriately? | ||
− | == | + | ==I/O== |
− | # Is the | + | # Are all resources closed after use (files, images… etc.)[using try…finally]? |
− | # Does the | + | # Is the read buffer of a resource appropriately sized? |
+ | # Does the reading of resources take into account memory usage [streaming]? | ||
+ | # Is I/O access minimized using an appropriate streaming mechanism? | ||
+ | |||
+ | ==Memory== | ||
+ | # Are intermediate objects, used in computations, scoped appropriately so they can be garbage collected quickly when no longer needed? | ||
+ | |||
+ | ==Methods== | ||
+ | # Are descriptive method names used in accord with naming conventions? | ||
+ | # Do all methods have appropriate access modifiers (private, protected, public)? | ||
+ | # Should any static methods be non-static, or any non-static methods be static? | ||
+ | # Do methods have a clear flow and return criteria? | ||
+ | # Do method names describe the entire functionality? | ||
+ | # Does the method provide one function? | ||
+ | # Is an empty set returned instead of null (Collections.empty…)? | ||
+ | # Avoid returning null, or, at a minimum, document (in the interface if applicable) methods that may return null. | ||
+ | |||
+ | ==Modularity== | ||
+ | # Do modules have a low level of coupling between them (methods and classes)? | ||
+ | # Do modules have a high level of cohesion within each one (methods or class)? | ||
+ | # Is dependency injection used instead of singletons? (singletons hide dependencies) | ||
+ | |||
+ | ==Performance== | ||
+ | # Can better data structures or more efficient algorithms be used? | ||
+ | # Are logical tests arranged such that the often successful and inexpensive tests precede the more pensive and less frequently successful tests? | ||
+ | # Can the cost of re-computing a value be reduced by computing it once and storing the results? | ||
+ | # Is every result that is computed and stored actually used? | ||
+ | # Can a computation be moved outside a loop? | ||
+ | # Do all tests within a loop need to be performed? | ||
+ | # Can multiple loops, operating on the same data, be combined into one loop? | ||
+ | |||
+ | ==Redundant Code== | ||
+ | # Are any variables or attributes redundant or unused? | ||
+ | # Does the code contain any uncalled or unneeded methods or leftover code stubs? | ||
+ | # Can any code be replaced by calls to external reusable objects? | ||
+ | # Are any blocks of code repeated that could be condensed into a single method? | ||
+ | |||
+ | ==Variables== | ||
+ | # Are descriptive variable and constant names used in accord with naming conventions (camel)? | ||
+ | # Are all variables properly defined with meaningful, consistent, and clear names? | ||
+ | # Is every variable correctly typed? | ||
+ | # Is every variable properly initialized? | ||
+ | # Should any of the variables be constants (final)? | ||
+ | # Should any of the fields be local variables? | ||
+ | # Do all fields have appropriate access modifiers (private, protected, public)? | ||
+ | # Should any static fields be non-static, or any non-static fields be static? | ||
+ | # Do any variables have confusingly similar names as other variables? | ||
+ | # Are variables defined close to where they are used? | ||
+ | # Are variables used defensively (add link)? | ||
[[Category:OSEE]] | [[Category:OSEE]] |
Revision as of 14:00, 26 October 2017
Contents
Arithmetic
- Are divisors tested for zero?
- Do comparisons of floating point numbers have tolerance?
Change Management
- Does the commit have a single focus?
- Is the commit too complicated to review effectively?
Classes
- Do any subclasses have common members that should be in the superclass?
- Can the class inheritance hierarchy be simplified?
- Are any modules excessively complex and need to be restructured or decomposed?
- Do class names reflect the functionality they provide?
- Does the class have a clear singular responsibility?
- Does the class have a clear lifecycle?
Code Style/Formatting
- Has the standard formatter been applied?
- Are methods and classes within size guidelines?
- Are coding guidelines checked with tools?
Comments
- Do the comments help understand the code?
- Are all comments consistent with the code?
- Do the commit comments follow the convention?
- Does the code contain sufficient, descriptive, and well written comments?
- Are comments only used for technical notes?
- Are any code segments commented out?
Concurrency
- Are there mutable member variables in classes with static methods that can be called concurrently?
Configuration
- Is the constant or configurable item acquired by a centralized object?
- Does the item need to be configurable?
Control Flow
- Are all for-loop control variables declared in the loop header?
- Is the best choice of looping constructs used?
- Will all loops terminate?
- When multiple exits from a loop are used, is each exit necessary and handled properly?
- Does each switch statement have a default case?
- Are missing switch case break statements correct and marked with a comment?
- Is the nesting of loops and branches too deep, and is it correct?
- Are conditionals checking for the positive?
- When ‘goto’ statements (continue, break, or multiple returns) are used, is each necessary and handled properly?
- Is polymorphism used to operate on items using a single if/else/switch to construct polymorphic objects instead of using if/else/switch each time we want to manipulate items of that type?
Exception Handling
- Are caught exceptions handled in a meaningful way?
- When exceptions are swallowed, is the exceptional case completely resolved? Was the system left in a consistent state?
- Are Exceptions only used for signaling error conditions and not flow control, which causes bad performance?
- Are runtime and checked exceptions and throwable java.lang.error considered and handled appropriately?
I/O
- Are all resources closed after use (files, images… etc.)[using try…finally]?
- Is the read buffer of a resource appropriately sized?
- Does the reading of resources take into account memory usage [streaming]?
- Is I/O access minimized using an appropriate streaming mechanism?
Memory
- Are intermediate objects, used in computations, scoped appropriately so they can be garbage collected quickly when no longer needed?
Methods
- Are descriptive method names used in accord with naming conventions?
- Do all methods have appropriate access modifiers (private, protected, public)?
- Should any static methods be non-static, or any non-static methods be static?
- Do methods have a clear flow and return criteria?
- Do method names describe the entire functionality?
- Does the method provide one function?
- Is an empty set returned instead of null (Collections.empty…)?
- Avoid returning null, or, at a minimum, document (in the interface if applicable) methods that may return null.
Modularity
- Do modules have a low level of coupling between them (methods and classes)?
- Do modules have a high level of cohesion within each one (methods or class)?
- Is dependency injection used instead of singletons? (singletons hide dependencies)
Performance
- Can better data structures or more efficient algorithms be used?
- Are logical tests arranged such that the often successful and inexpensive tests precede the more pensive and less frequently successful tests?
- Can the cost of re-computing a value be reduced by computing it once and storing the results?
- Is every result that is computed and stored actually used?
- Can a computation be moved outside a loop?
- Do all tests within a loop need to be performed?
- Can multiple loops, operating on the same data, be combined into one loop?
Redundant Code
- Are any variables or attributes redundant or unused?
- Does the code contain any uncalled or unneeded methods or leftover code stubs?
- Can any code be replaced by calls to external reusable objects?
- Are any blocks of code repeated that could be condensed into a single method?
Variables
- Are descriptive variable and constant names used in accord with naming conventions (camel)?
- Are all variables properly defined with meaningful, consistent, and clear names?
- Is every variable correctly typed?
- Is every variable properly initialized?
- Should any of the variables be constants (final)?
- Should any of the fields be local variables?
- Do all fields have appropriate access modifiers (private, protected, public)?
- Should any static fields be non-static, or any non-static fields be static?
- Do any variables have confusingly similar names as other variables?
- Are variables defined close to where they are used?
- Are variables used defensively (add link)?