Code Review

advanced105 min

Introduction to Code Review

Code review is a systematic process of examining code to find bugs, improve quality, and ensure consistency. For FTC teams, code reviews help catch issues early and improve team collaboration. This lesson covers how to conduct effective code reviews for robot programming.

Why Code Review Matters for FTC

  • Catches bugs before they reach the robot during competition
  • Improves code quality and maintainability
  • Helps team members learn from each other
  • Ensures safety and performance standards are met
  • Maintains consistent coding standards across the team

Code Review Process for FTC Teams

Establishing a systematic code review process helps ensure all code changes are properly examined. A good review process includes checklists, feedback guidelines, and clear communication.

Code Review Checklist - Review Categories

Main review categories for FTC code

Good Practices:

  • Functionality - Ensure code works correctly and handles edge cases
  • Safety - Prevent robot damage and ensure safe operation
  • Performance - Maintain smooth robot operation and efficiency
  • Style - Improve code readability and maintainability
  • Documentation - Ensure code is understandable and well-documented

Avoid:

  • Reviewing only one category at a time
  • Skipping safety checks in favor of functionality
  • Ignoring performance issues for cleaner code
  • Focusing only on style without checking functionality
  • Documenting without verifying code correctness

Understanding Review Categories

The review categories help organize the code review process. Functionality checks ensure the code works correctly, safety checks prevent robot damage, performance checks maintain smooth operation, style checks improve readability, and documentation checks ensure maintainability.

Code Review Checklist - Functionality and Safety Checks

Critical categories for FTC code reviews

Good Practices:

  • Does the code do what it’s supposed to do?
  • Are all edge cases handled?
  • Does the code handle hardware failures gracefully?
  • Are there any infinite loops or deadlocks?
  • Does the autonomous sequence complete successfully?
  • Are gamepad controls intuitive and responsive?
  • Does the robot stop when it should?
  • Are there safety timeouts for all operations?
  • Does the robot stop if sensors detect obstacles?
  • Are motor powers limited to safe values?
  • Is there emergency stop functionality?
  • Does the code prevent the robot from damaging itself?
  • Are there checks for hardware initialization failures?
  • Does the robot have a safe default state?

Avoid:

  • Assuming hardware will always work correctly
  • Ignoring edge cases for simpler code
  • Using unlimited motor powers
  • Missing emergency stop functionality
  • Not testing autonomous sequences thoroughly
  • Overlooking sensor failure scenarios
  • Forgetting to handle initialization errors

Understanding Functionality and Safety Checks

Functionality checks ensure your robot code works as intended and handles real-world scenarios. Safety checks are critical for preventing robot damage and ensuring safe operation during competition. These categories should be reviewed first as they directly impact robot performance and safety.

Code Review Checklist - Performance and Style Checks

Focus on code efficiency and maintainability

Good Practices:

  • Does the main loop run at acceptable speed (50Hz)?
  • Are there any memory leaks or excessive object creation?
  • Are sensor readings optimized (not read too frequently)?
  • Are complex calculations broken into smaller pieces?
  • Is telemetry updated at reasonable intervals?
  • Are there any blocking operations in the main loop?
  • Is the code efficient for the target hardware?
  • Are variable and method names descriptive?
  • Is the code properly indented and formatted?
  • Are there consistent naming conventions?
  • Is the code organized into logical methods?
  • Are magic numbers replaced with named constants?
  • Is the code readable and well-structured?
  • Are there appropriate comments for complex logic?

Avoid:

  • Creating objects in tight loops
  • Reading sensors every loop iteration
  • Updating telemetry too frequently
  • Using blocking operations in main loop
  • Using unclear variable names
  • Leaving magic numbers in code
  • Writing overly complex methods
  • Forgetting to comment complex logic

Performance and Style Considerations

Performance checks ensure your robot operates smoothly and efficiently. Style checks improve code readability and maintainability, making it easier for team members to understand and modify the code. Documentation checks ensure that complex logic is properly explained for future reference.

Code Review Checklist - Documentation Checks

Ensure code is maintainable and understandable

Good Practices:

  • Is there JavaDoc for public methods?
  • Are complex algorithms explained with comments?
  • Is there a README file for the project?
  • Are hardware configurations documented?
  • Are there comments explaining robot behavior?
  • Is the code self-documenting where possible?
  • Are there inline comments for non-obvious code?

Avoid:

  • Writing code without any comments
  • Using unclear method names
  • Not documenting hardware requirements
  • Leaving complex logic unexplained
  • Forgetting to update documentation
  • Using outdated comments
  • Not explaining robot behavior decisions

Understanding Documentation Checks

Documentation checks ensure that your code is maintainable and understandable by other team members. Good documentation includes JavaDoc comments, inline explanations for complex logic, and project-level documentation that explains the overall robot design and configuration.

Example of Well-Reviewed OpMode

Let’s look at an example of a well-reviewed OpMode that follows the checklist. This example demonstrates proper code organization, safety measures, and documentation.

Reviewed OpMode - Constants and Hardware Components

Example of a well-reviewed OpMode that follows the checklist. This class demonstrates proper organization and safety practices:

    /**
     * Example of a well-reviewed OpMode that follows the checklist
     */
    public static class ReviewedOpMode extends OpMode {
        
        // Constants - no magic numbers
        private static final double MAX_DRIVE_SPEED = 0.8;
        private static final double MAX_TURN_SPEED = 0.6;
        private static final double DEADBAND_THRESHOLD = 0.1;
        private static final double SAFETY_DISTANCE_CM = 10.0;
        private static final long TELEMETRY_UPDATE_INTERVAL_MS = 100;
        
        // Hardware components - clearly named
        private DcMotor leftDriveMotor, rightDriveMotor;
        private Servo armServo, clawServo;
        private ColorSensor gameElementDetector;
        private DistanceSensor proximitySensor;
        
        // State variables - descriptive names
        private boolean isEmergencyStopActive = false;
        private long lastTelemetryUpdate = 0;
        private double currentArmPosition = 0.5;
        private boolean isClawOpen = false;

Understanding Well-Reviewed OpMode Structure

The well-reviewed OpMode uses named constants instead of magic numbers, has clearly named hardware components, and includes descriptive state variables. This makes the code more readable and maintainable.

Reviewed OpMode - Initialization Methods

Add initialization methods with proper error handling and documentation:

        /**
         * Initializes the OpMode and all hardware components.
         * Includes safety checks and error handling.
         */
        @Override
        public void init() {
            try {
                initializeHardware();
                configureHardware();
                resetRobotState();
                displayInitializationStatus();
            } catch (Exception e) {
                handleInitializationError(e);
            }
        }
        
        /**
         * Main control loop with safety and performance considerations.
         * Runs at approximately 50Hz for smooth robot control.
         */
        @Override
        public void loop() {
            // Check for emergency stop first
            if (isEmergencyStopActive) {
                stopAllMotors();
                return;
            }
            
            // Process input with safety checks
            processDriverInput();
            
            // Update robot state
            updateRobotState();
            
            // Apply safety checks
            applySafetyChecks();
            
            // Update telemetry periodically for performance
            updateTelemetryIfNeeded();
        }

Understanding Initialization and Main Loop

The initialization method includes proper error handling with try-catch blocks. The main loop is organized into clear sections with safety checks first, followed by input processing, state updates, and periodic telemetry updates for performance.

Reviewed OpMode - Hardware Initialization and Configuration

Add hardware initialization and configuration methods with proper error handling:

        /**
         * Initializes all hardware components with error handling.
         * Throws exception if critical hardware is missing.
         */
        private void initializeHardware() throws Exception {
            // Initialize drive motors
            leftDriveMotor = hardwareMap.get(DcMotor.class, "left_drive");
            rightDriveMotor = hardwareMap.get(DcMotor.class, "right_drive");
            
            // Initialize servos
            armServo = hardwareMap.get(Servo.class, "arm_servo");
            clawServo = hardwareMap.get(Servo.class, "claw_servo");
            
            // Initialize sensors
            gameElementDetector = hardwareMap.get(ColorSensor.class, "color_sensor");
            proximitySensor = hardwareMap.get(DistanceSensor.class, "distance_sensor");
            
            // Verify critical hardware is available
            if (leftDriveMotor == null || rightDriveMotor == null) {
                throw new Exception("Critical drive motors not found");
            }
        }
        
        /**
         * Configures hardware settings for safe operation.
         */
        private void configureHardware() {
            // Configure drive motors for safety
            leftDriveMotor.setDirection(DcMotor.Direction.FORWARD);
            rightDriveMotor.setDirection(DcMotor.Direction.REVERSE);
            leftDriveMotor.setZeroPowerBehavior(DcMotor.ZeroPowerBehavior.BRAKE);
            rightDriveMotor.setZeroPowerBehavior(DcMotor.ZeroPowerBehavior.BRAKE);
            
            // Set initial servo positions
            if (armServo != null) armServo.setPosition(currentArmPosition);
            if (clawServo != null) clawServo.setPosition(isClawOpen ? 1.0 : 0.0);
        }

Understanding Hardware Initialization and Configuration

The hardware initialization method includes null checks for critical components and throws exceptions if essential hardware is missing. The configuration method sets up safe default behaviors like brake mode for motors and initial servo positions.

Common FTC-Specific Code Issues

FTC code has specific patterns and common issues that reviewers should look for. Understanding these patterns helps identify potential problems before they cause issues during competition.

Common Issues - Deadband and Timeout Examples

Examples of common FTC code issues and how to fix them. This section covers deadband and timeout problems:

// Common FTC Code Issues and Solutions

public class CommonIssuesExamples {
    
    /**
     * ISSUE 1: No deadband on gamepad input
     * PROBLEM: Controller drift causes unwanted movement
     * SOLUTION: Apply deadband to prevent drift
     */
    public static class DeadbandExample {
        // BAD - No deadband
        public void badGamepadProcessing() {
            double forward = gamepad1.left_stick_y;
            double turn = gamepad1.right_stick_x;
            // This can cause unwanted movement from controller drift
        }
        
        // GOOD - With deadband
        public void goodGamepadProcessing() {
            double forward = applyDeadband(gamepad1.left_stick_y);
            double turn = applyDeadband(gamepad1.right_stick_x);
            // Prevents unwanted movement from controller drift
        }
        
        private double applyDeadband(double input) {
            return Math.abs(input) < 0.1 ? 0.0 : input;
        }
    }

Understanding Deadband Issues

Controller drift is a common problem in FTC. Without a deadband, small controller movements can cause unwanted robot movement. The applyDeadband method filters out small inputs below the threshold, preventing drift-related issues.

Common Issues - Timeout Examples

Add timeout examples to prevent robot operations from getting stuck:

    /**
     * ISSUE 2: No safety timeouts
     * PROBLEM: Robot can get stuck in operations
     * SOLUTION: Add timeouts for all operations
     */
    public static class TimeoutExample {
        private double operationStartTime;
        private static final double OPERATION_TIMEOUT = 5.0; // 5 seconds
        
        // BAD - No timeout
        public void badOperation() {
            while (!isOperationComplete()) {
                // Robot can get stuck here forever
                performOperation();
            }
        }
        
        // GOOD - With timeout
        public void goodOperation() {
            operationStartTime = getRuntime();
            while (!isOperationComplete() && !isTimeoutReached()) {
                performOperation();
            }
            
            if (isTimeoutReached()) {
                handleTimeout();
            }
        }
        
        private boolean isTimeoutReached() {
            return getRuntime() - operationStartTime > OPERATION_TIMEOUT;
        }
        
        private void handleTimeout() {
            // Stop operation and notify driver
            stopOperation();
            telemetry.addData("Warning", "Operation timed out");
        }
    }

Understanding Timeout Issues

Timeouts prevent robots from getting stuck in operations indefinitely. The good example tracks operation start time and checks for timeout conditions, ensuring the robot can recover from stuck operations and notify the driver of issues.

Common Issues - Hardware Failure and Telemetry Examples

Add examples for hardware failure handling and telemetry optimization:

    /**
     * ISSUE 3: No hardware failure handling
     * PROBLEM: Code crashes if hardware is missing or fails
     * SOLUTION: Check for null and handle exceptions
     */
    public static class HardwareFailureExample {
        private DcMotor driveMotor;
        
        // BAD - No null check
        public void badHardwareAccess() {
            driveMotor = hardwareMap.get(DcMotor.class, "drive_motor");
            driveMotor.setPower(0.5); // Crashes if motor is null
        }
        
        // GOOD - With null check
        public void goodHardwareAccess() {
            driveMotor = hardwareMap.get(DcMotor.class, "drive_motor");
            if (driveMotor != null) {
                driveMotor.setPower(0.5);
            } else {
                telemetry.addData("ERROR", "Drive motor not found");
            }
        }
    }

Understanding Hardware Failure Handling

Hardware failures are common in FTC due to loose connections or missing components. The good example includes null checks to prevent crashes and provides feedback to the driver when hardware is missing.

Common Issues - Telemetry Optimization

Add telemetry optimization examples to improve performance:

    /**
     * ISSUE 4: Inefficient telemetry updates
     * PROBLEM: Too much telemetry slows down the main loop
     * SOLUTION: Update telemetry periodically
     */
    public static class TelemetryExample {
        private long lastTelemetryUpdate = 0;
        private static final long TELEMETRY_INTERVAL = 100; // 100ms
        
        // BAD - Updates every loop
        public void badTelemetryUpdate() {
            telemetry.addData("Runtime", getRuntime());
            telemetry.addData("Motor Power", motor.getPower());
            telemetry.addData("Sensor Value", sensor.getValue());
            // This runs every 20ms, which is excessive
        }
        
        // GOOD - Updates periodically
        public void goodTelemetryUpdate() {
            long currentTime = System.currentTimeMillis();
            if (currentTime - lastTelemetryUpdate > TELEMETRY_INTERVAL) {
                telemetry.addData("Runtime", getRuntime());
                telemetry.addData("Motor Power", motor.getPower());
                telemetry.addData("Sensor Value", sensor.getValue());
                lastTelemetryUpdate = currentTime;
            }
        }
    }
}

Understanding Telemetry Optimization

Excessive telemetry updates can slow down the main loop and reduce robot responsiveness. The good example updates telemetry only periodically (every 100ms) instead of every loop iteration, maintaining smooth robot control while still providing useful feedback.

Performance and Safety Review Criteria

Performance and safety are critical for FTC robots. Reviewers must ensure code meets performance requirements and includes proper safety measures.

Performance Review Criteria

  • Main loop runs at 50Hz (20ms per loop) or faster
  • No blocking operations in the main loop
  • Telemetry updates are limited to reasonable frequency
  • Memory usage is optimized (no excessive object creation)
  • Sensor readings are not performed too frequently
  • Complex calculations are broken into smaller pieces

Safety Review Criteria

  • All operations have timeout limits
  • Emergency stop functionality is implemented
  • Motor powers are limited to safe values
  • Proximity sensors are used to prevent collisions
  • Hardware failures are handled gracefully
  • Robot has a safe default state when stopped

Collaborative Development Workflows

Effective collaboration requires clear workflows, communication, and conflict resolution strategies. Teams need to work together efficiently while maintaining code quality.

Git Workflow for Team Collaboration

Implement a collaborative Git workflow for FTC teams. This example shows how to structure your project with version control in mind:

// Git Workflow for FTC Team Collaboration

/*
 * BRANCHING STRATEGY FOR FTC TEAMS
 * 
 * Main Branch Structure:
 * - main: Production-ready code for competition
 * - development: Integration branch for testing
 * - feature/*: Individual feature branches
 * - hotfix/*: Emergency fixes for competition
 * 
 * Workflow:
 * 1. Create feature branch from development
 * 2. Develop and test feature
 * 3. Create pull request to development
 * 4. Code review and testing
 * 5. Merge to development
 * 6. Integration testing
 * 7. Merge to main for competition
 * 
 * Example Commands:
 * 
 * # Start new feature
 * git checkout development
 * git pull origin development
 * git checkout -b feature/autonomous-collector
 * 
 * # Develop feature
 * # ... make changes ...
 * git add .
 * git commit -m "Add autonomous collector functionality"
 * git push origin feature/autonomous-collector
 * 
 * # Create pull request (on GitHub/GitLab)
 * # Review and merge to development
 * 
 * # Prepare for competition
 * git checkout main
 * git merge development
 * git tag -a v1.0 -m "Competition version 1.0"
 * git push origin main --tags
 */

Understanding Git Workflow

The Git workflow provides a structured approach to team collaboration. Feature branches allow individual development, pull requests enable code review, and the main branch contains only competition-ready code. This ensures code quality and team coordination.

Practice: Code Review Exercise

Practice code review skills with these exercises:

  • Review a provided OpMode using the checklist
  • Identify and fix common FTC code issues
  • Create a well-structured OpMode following best practices
  • Conduct a peer review of your code
  • Document your review findings
// Sample OpMode to review:
import com.qualcomm.robotcore.eventloop.opmode.OpMode;
import com.qualcomm.robotcore.eventloop.opmode.TeleOp;
import com.qualcomm.robotcore.hardware.DcMotor;
import com.qualcomm.robotcore.hardware.Servo;

@TeleOp(name = "SampleOpMode")
public class SampleOpMode extends OpMode {
    
    // Hardware components
    private DcMotor leftMotor, rightMotor;
    private Servo armServo;
    
    // State variables
    private boolean isArmUp = false;
    private double driveSpeed = 0.8;
    
    @Override
    public void init() {
        leftMotor = hardwareMap.get(DcMotor.class, "left_motor");
        rightMotor = hardwareMap.get(DcMotor.class, "right_motor");
        armServo = hardwareMap.get(Servo.class, "arm_servo");
    }
    
    @Override
    public void loop() {
        // Drive control
        double forward = gamepad1.left_stick_y;
        double turn = gamepad1.right_stick_x;
        
        double leftPower = forward + turn;
        double rightPower = forward - turn;
        
        leftMotor.setPower(leftPower);
        rightMotor.setPower(rightPower);
        
        // Arm control
        if (gamepad1.a) {
            armServo.setPosition(1.0);
            isArmUp = true;
        } else if (gamepad1.b) {
            armServo.setPosition(0.0);
            isArmUp = false;
        }
        
        // Telemetry
        telemetry.addData("Left Power", leftPower);
        telemetry.addData("Right Power", rightPower);
        telemetry.addData("Arm Up", isArmUp);
        telemetry.update();
    }
}

Additional Resources

<a href="https://google.github.io/eng-practices/review/" target="_blank">Google Code Review Guidelines</a><a href="https://git-scm.com/book/en/v2/Git-Branching-Branches-in-a-Nutshell" target="_blank">Git Branching</a>