Clarity
|
Was the code refractured to make it clear and easy to understand?
|
Maintainability
|
Will other programmers be able to maintain this code?
|
Is it well-commented and documented properly?
|
Accuracy
|
Does the code accomplish what it is meant to do?
|
Reliability and Robustness
|
Is the code fault-tolerant? Is it error-tolerant?
|
Will it handle abnormal conditions or malformed input?
|
Does it fail gracefully if it encounters an unexpected condition?
|
Does the code account for null pointer exceptions?
|
Security
|
Is the code vulnerable to unauthorized access or malicious use or modification?
|
Scalability
|
Could the code be a bottleneck that prevents the system from growing to accommodate increased load, data, users or input?
|
Reusability
|
Could this code be reused in other applications?
|
Can it be made more general?
|
Efficiency
|
Does the code make efficient use of memory, CPU cycles, bandwidth or other system resources?
|
Is the Code optimized?
|
Functionality
|
Is the code written correctly to suffice the functional requirements?
|
Does the code in line with the design?
|
Are there any requirements of design that were not implemented?
|
Variable and Constant Declaration
|
Are descriptive variable and constant names used in accord with naming conventions?
|
Is every variable properly initialized?
|
Are there variables that should be constants?
|
Are there attributes that should be local variables? Is Data need to be persistant?
|
Do all attributes have appropriate access modifiers (private, protected, public)?
|
Are there static attributes that should be non-static or vice-versa?
|
Are all variables properly defined with meaningful, consistent, and clear names?
|
Are there variables with confusingly similar names?
|
Are there any redundant/unused variables or attributes?
|
Method Definition
|
Are there any uncalled or unneeded methods?
|
Are descriptive method names used in accord with naming conventions?
|
Do all methods have appropriate access modifiers (private, protected, public)?
|
Are any modules excessively complex and should be restructured or split into multiple routines?
|
Are there any blocks of repeated code that could be condensed into a single method?
|
Is TRY..CATCH Block used appropriately? Example: External Calls
|
Is test case written for the method?
|
Is all scenarios covered in test case?
|
Computation/Numeric
|
Are there any computations with mixed data types? Example bool and int
|
For each expression with more than one operator: Are the order of evaluation and precedence correct?
|
Are parentheses used to avoid ambiguity?
|
Has each Boolean expression been simplified by driving negations inward?
|
Is string trimmed before checking for length or string comparison?
|
Is Boundary condition checked properly (> or >=)?
|
Does the code uses correct precision in comparing double values?
|
Unit Tests
|
Is there any tests and functional tests written? What is the coverage?
|
Playbook Impact
|
Requires changes to playbook
|
Java Code Review Checklist
Clean Code
Checklist Item
|
Category
|
Use Intention-Revealing Names
|
Meaningful Names
|
Pick one word per concept
|
Meaningful Names
|
Use Solution/Problem Domain Names
|
Meaningful Names
|
Classes should be small!
|
Classes
|
Functions should be small!
|
Functions
|
Do one Thing
|
Functions
|
Don't Repeat Yourself (Avoid Duplication)
|
Functions
|
Explain yourself in code
|
Comments
|
Make sure the code formatting is applied
|
Formatting
|
Use Exceptions rather than Return codes
|
Exceptions
|
Don't return Null
|
Exceptions
|
Security
Checklist Item
|
Category
|
Make class final if not being used for inheritance
|
Fundamentals
|
Avoid duplication of code
|
Fundamentals
|
Restrict privileges: Application to run with the least privilege mode required for functioning
|
Fundamentals
|
Minimize the accessibility of classes and members
|
Fundamentals
|
Document security related information
|
Fundamentals
|
Input into a system should be checked for valid data size and range
|
Denial of Service
|
Avoid excessive logs for unusual behavior
|
Denial of Service
|
Release resources (Streams, Connections, etc) in all cases
|
Denial of Service
|
Purge sensitive information from exceptions (exposing file path, internals of the system, configuration)
|
Confidential Information
|
Do not log highly sensitive information
|
Confidential Information
|
Consider purging highly sensitive from memory after use
|
Confidential Information
|
Avoid dynamic SQL, use prepared statement
|
Injection Inclusion
|
Limit the accessibility of packages,classes, interfaces, methods, and fields
|
Accessibility Extensibility
|
Limit the extensibility of classes and methods (by making it final)
|
Accessibility Extensibility
|
Validate inputs (for valid data, size, range, boundary conditions, etc)
|
Input Validation
|
Validate output from untrusted objects as input
|
Input Validation
|
Define wrappers around native methods (not declare a native method public)
|
Input Validation
|
Treat output from untrusted object as input
|
Mutability
|
Make public static fields final (to avoid caller changing the value)
|
Mutability
|
Avoid exposing constructors of sensitive classes
|
Object Construction
|
Avoid serialization for security-sensitive classes
|
Serialization Deserialization
|
Guard sensitive data during serialization
|
Serialization Deserialization
|
Be careful caching results of potentially privileged operations
|
Serialization Deserialization
|
Only use JNI when necessary
|
Access Control
|
Reference for security - http://www.oracle.com/technetwork/java/seccodeguide-139067.html
Performance
Checklist Item
|
Category
|
Avoid excessive synchronization
|
Concurrency
|
Keep Synchronized Sections Small
|
Concurrency
|
Beware the performance of string concatenation
|
General Programming
|
Avoid creating unnecessary objects
|
Creating and Destroying Objects
|
General
Category
|
Checklist Item
|
Use checked exceptions for recoverable conditions and runtime exceptions for programming errors
|
Exceptions
|
Favor the use of standard exceptions
|
Exceptions
|
Don't ignore exceptions
|
Exceptions
|
Check parameters for validity
|
Methods
|
Return empty arrays or collections, not nulls
|
Methods
|
Minimize the accessibility of classes and members
|
Classes and Interfaces
|
In public classes, use accessor methods, not public fields
|
Classes and Interfaces
|
Minimize the scope of local variables
|
General Programming
|
Refer to objects by their interfaces
|
General Programming
|
Adhere to generally accepted naming conventions
|
General Programming
|
Avoid finalizers
|
Creating and Destroying Objects
|
Always override hashCode when you override equals
|
General Programming
|
Always override toString
|
General Programming
|
Use enums instead of int constants
|
Enums and Annotations
|
Use marker interfaces to define types
|
Enums and Annotations
|
Synchronize access to shared mutable data
|
Concurrency
|
Prefer executors to tasks and threads
|
Concurrency
|
Document thread safety
|
Concurrency
|
Valid JUnit / JBehave test cases exist
|
Testing
|
Static Code Analysis
Category
|
Checklist Item
|
Check static code analyzer report for the classes added/modified
|
Static Code Analysis
|
No comments:
Post a Comment