2.1: Clean Code Principles

Clean Code for Embedded C

Why Clean Code Matters in Safety-Critical Systems

Industry Data:

  • Defect Cost: Fixing a defect in production: €10,000–100,000 (field recall); fixing it during code review: €50–200
  • Readability: Engineers spend 70% of their time reading code and 30% writing it
  • Maintenance: 60–80% of software lifecycle cost is maintenance (understanding and modifying code)

ASPICE Requirement:

  • SWE.3 BP2: "Software units are designed according to coding standards" (MISRA C:2012 for automotive)

Principle 1: Meaningful Names

Variable Names

Bad Names (cryptic, requires mental mapping):

int d;      /* What is d? Days? Distance? Delta? */
int tmp;    /* Temporary what? */
int x, y;   /* What do x and y represent? */

Good Names (self-documenting):

int distance_meters;           /* Clear unit */
int radar_timeout_ms;          /* Clear purpose and unit */
int num_active_sensors;        /* Clear meaning */

Naming Rules:

  1. Use intention-revealing names: Name should answer "why it exists, what it does"
  2. Avoid abbreviations (unless standard: ms for milliseconds, km/h for kilometers per hour)
  3. Include units for physical quantities: _meters, _kmh, _ms
  4. Use pronounceable names: radar_timeout_ms (good) vs rdrtmo (bad)

Example: Sensor Fusion

/* Bad: Cryptic names */
float calc(float r, float c) {
    float w1 = 0.6;
    float w2 = 0.4;
    return r * w1 + c * w2;
}

/* Good: Self-documenting */
float CalculateFusedDistance(float radar_distance_m, float camera_distance_m) {
    const float RADAR_WEIGHT = 0.6;   /* Radar more reliable */
    const float CAMERA_WEIGHT = 0.4;  /* Camera less reliable in rain */

    return (radar_distance_m * RADAR_WEIGHT) +
           (camera_distance_m * CAMERA_WEIGHT);
}

Function Names

Bad Names (vague verbs):

void Process(void);        /* Process what? */
void DoStuff(void);        /* What stuff? */
void Handle(int data);     /* Handle how? */

Good Names (specific verb + noun):

void ProcessRadarData(void);           /* Clear what's processed */
void CalculateTargetSpeed(void);       /* Clear action + result */
void ValidateSensorData(int* data);    /* Clear purpose */

Naming Convention (AUTOSAR C++ Coding Guidelines adapted for C):

  • Functions: UpperCamelCase or Module_VerbNoun (e.g., ACC_CalculateSpeed)
  • Variables: snake_case or lowerCamelCase (e.g., target_speed or targetSpeed)
  • Constants: UPPER_SNAKE_CASE (e.g., MAX_SPEED_KMH)
  • Types: UpperCamelCase_t (e.g., SensorData_t)

Example: ACC Speed Control

/**
 * @brief Calculate target speed based on obstacle distance
 * @implements [SWE-045-3] Target Speed Calculation
 */
float ACC_CalculateTargetSpeed(float obstacle_distance_m) {
    const float MIN_SAFE_DISTANCE_M = 10.0;  /* 10 meters minimum */
    const float DECEL_RATE_KMH = 5.0;        /* Reduce 5 km/h per second */

    if (obstacle_distance_m < MIN_SAFE_DISTANCE_M) {
        return g_current_speed_kmh - DECEL_RATE_KMH;
    }

    return g_set_speed_kmh;
}

Principle 2: Small Functions

Function Size Guidelines

Rule: Functions should be 5–15 lines (excluding comments), with a maximum of 50 lines including comments and whitespace.

Bad Function (too long, multiple responsibilities):

void ACC_ControlLoop(void) {
    /* 100 lines of mixed logic:
     * - Read CAN messages
     * - Sensor fusion math
     * - PID controller
     * - Actuator commands
     * - Error handling
     */
}

[FAIL] Problems:

  • Hard to understand (cognitive overload)
  • Hard to test (requires mocking entire system)
  • Hard to reuse (everything coupled)

Good Functions (small, single responsibility):

/**
 * @brief Main control loop for ACC system
 */
void ACC_ControlLoop(void) {
    float obstacle_distance = SensorFusion_GetObstacleDistance();
    float target_speed = ACC_CalculateTargetSpeed(obstacle_distance);
    ACC_ApplyControl(target_speed);
}

/**
 * @brief Get fused obstacle distance from radar and camera
 * @implements [SWE-045-1] Sensor Fusion
 */
float SensorFusion_GetObstacleDistance(void) {
    float radar_distance = CAN_ReadRadarDistance();
    float camera_distance = Ethernet_ReadCameraDistance();

    /* Weighted average: Radar 60%, Camera 40% */
    return (radar_distance * 0.6F) + (camera_distance * 0.4F);
}

/**
 * @brief Calculate target speed based on obstacle distance
 * @implements [SWE-045-3] Target Speed Calculation
 */
float ACC_CalculateTargetSpeed(float obstacle_distance) {
    const float SAFE_DISTANCE = g_vehicle_speed * 2.0F;  /* 2 seconds */

    if (obstacle_distance < SAFE_DISTANCE) {
        return g_vehicle_speed - 5.0F;  /* Decelerate */
    }

    return g_set_speed;  /* Maintain set speed */
}

/**
 * @brief Apply control commands to actuators
 * @implements [SWE-045-4] Actuator Control
 */
void ACC_ApplyControl(float target_speed) {
    if (target_speed < g_vehicle_speed) {
        Brake_Apply(/* calculate brake force */);
    } else if (target_speed > g_vehicle_speed) {
        Throttle_Apply(/* calculate throttle */);
    }
}

[PASS] Benefits:

  • Each function 5-10 lines (easy to understand at a glance)
  • Each function has one clear responsibility
  • Easy to unit test (pure functions, minimal dependencies)
  • Easy to reuse (modular components)

Extract Till You Drop

Strategy: If a function has multiple sections (separated by blank lines or comments), extract each section into a function

Before (sections in one function):

void ProcessSensorData(void) {
    /* Section 1: Read sensors */
    int radar_raw = CAN_ReadRadar();
    int camera_raw = Ethernet_ReadCamera();

    /* Section 2: Validate data */
    if (radar_raw < 0 || radar_raw > 200) {
        radar_raw = RADAR_INVALID;
    }
    if (camera_raw < 0 || camera_raw > 200) {
        camera_raw = CAMERA_INVALID;
    }

    /* Section 3: Fuse data */
    int fused = (radar_raw * 0.6) + (camera_raw * 0.4);

    /* Section 4: Apply to control */
    UpdateTargetSpeed(fused);
}

After (extracted functions):

void ProcessSensorData(void) {
    SensorData_t sensors = ReadSensors();
    sensors = ValidateSensors(sensors);
    float fused_distance = FuseSensorData(sensors);
    UpdateTargetSpeed(fused_distance);
}

SensorData_t ReadSensors(void) {
    SensorData_t data;
    data.radar_distance_m = CAN_ReadRadar();
    data.camera_distance_m = Ethernet_ReadCamera();
    return data;
}

SensorData_t ValidateSensors(SensorData_t data) {
    const int MAX_VALID_DISTANCE = 200;

    if (data.radar_distance_m < 0 || data.radar_distance_m > MAX_VALID_DISTANCE) {
        data.radar_valid = false;
    }
    if (data.camera_distance_m < 0 || data.camera_distance_m > MAX_VALID_DISTANCE) {
        data.camera_valid = false;
    }

    return data;
}

float FuseSensorData(SensorData_t data) {
    if (!data.radar_valid && !data.camera_valid) {
        return SENSOR_FUSION_INVALID;
    }

    if (!data.radar_valid) {
        return data.camera_distance_m;  /* Use camera only */
    }

    if (!data.camera_valid) {
        return data.radar_distance_m;  /* Use radar only */
    }

    /* Both valid: Weighted average */
    return (data.radar_distance_m * 0.6F) + (data.camera_distance_m * 0.4F);
}

Principle 3: Do One Thing

Single Responsibility Principle (SRP)

Definition: A function should do one thing, at one level of abstraction

Violation Example (mixing abstraction levels):

void ACC_Update(void) {
    /* High-level logic */
    float distance = GetObstacleDistance();

    /* Low-level details (should be in separate function) */
    uint8_t can_buffer[8];
    CAN_ReadMessage(0x200, can_buffer);
    uint16_t raw_distance = (can_buffer[0] << 8) | can_buffer[1];
    float distance_m = raw_distance / 1000.0;

    /* Back to high-level logic */
    float target_speed = CalculateTargetSpeed(distance_m);
    ApplyControl(target_speed);
}

[FAIL] Problem: Mixes high-level (get distance, calculate, apply) with low-level (CAN byte manipulation)

Correct Example (consistent abstraction):

void ACC_Update(void) {
    /* All at same abstraction level (high-level operations) */
    float distance = GetObstacleDistance();
    float target_speed = CalculateTargetSpeed(distance);
    ApplyControl(target_speed);
}

float GetObstacleDistance(void) {
    /* Mid-level: Orchestrate sensor reading and fusion */
    float radar_distance = Radar_ReadDistance();
    float camera_distance = Camera_ReadDistance();
    return FuseDistances(radar_distance, camera_distance);
}

float Radar_ReadDistance(void) {
    /* Low-level: CAN byte manipulation */
    uint8_t can_buffer[8];
    CAN_ReadMessage(0x200, can_buffer);
    uint16_t raw_distance = (can_buffer[0] << 8) | can_buffer[1];
    return raw_distance / 1000.0F;  /* Convert mm to meters */
}

[PASS] Benefit: Each function operates at consistent abstraction level


Principle 4: Comments (Use Sparingly)

Good Comments

1. Doxygen Headers (required for all public functions):

/**
 * @brief Calculate PID controller output
 * @implements [SWE-045-9] PID Speed Control
 * @param[in] error_kmh Speed error (set_speed - current_speed) in km/h
 * @param[in] dt_sec Time since last update in seconds
 * @return Throttle/brake command [-100, +100] (negative = brake, positive = throttle)
 * @safety_class ASIL-B
 */
float PID_Calculate(float error_kmh, float dt_sec);

2. Requirement Traceability:

/**
 * @implements [SWE-045-10] Anti-Windup for Integral Term
 */
if (g_pid_integral > MAX_INTEGRAL) {
    g_pid_integral = MAX_INTEGRAL;  /* Clamp to prevent windup */
}

3. Safety Rationale (explain "why", not "what"):

/* ISO 26262 requires fail-safe: Default to 0 km/h (system off) on error */
if (sensor_fault_detected) {
    return 0.0F;
}

4. Non-Obvious Algorithms:

/* Kalman filter predict step: x = Ax + Bu */
x_pred = (A * x_est) + (B * u);

/* Kalman filter update step: K = P*H^T / (H*P*H^T + R) */
kalman_gain = (P * H_transpose) / ((H * P * H_transpose) + R);

Bad Comments (Delete These)

1. Redundant (says what code already says):

/* Bad: Redundant */
i++;  // Increment i

/* Good: No comment needed (obvious) */
i++;

2. Misleading (out of date, wrong):

/* Bad: Misleading (code changed, comment didn't) */
/* Calculate distance in miles */
float distance_m = radar_reading / 1000.0;  /* Actually in meters! */

/* Good: No comment, clear variable name */
float distance_m = radar_reading / 1000.0;

3. Journal (change history belongs in Git):

/* Bad: Journal in code */
/* 2023-05-12: Changed MAX_SPEED from 120 to 150 (John) */
/* 2023-08-03: Fixed bug in speed calculation (Alice) */
const int MAX_SPEED = 150;

/* Good: No comment, use git log */
const int MAX_SPEED = 150;

4. Closing Brace Comments (function too long if you need these):

/* Bad: Closing brace comments (function too long) */
void LongFunction(void) {
    if (condition1) {
        for (int i = 0; i < 100; i++) {
            /* ... 50 lines ... */
        }  /* end for */
    }  /* end if */
}  /* end LongFunction */

/* Good: Refactor into small functions (no closing comments needed) */
void ProcessCondition1(void) {
    /* Process in small, focused function */
    for (int i = 0; i < 100; i++) {
        ProcessSingleItem(i);
    }
}

void ProcessSingleItem(int index) {
    /* Each function is small and clear */
    /* No closing brace comments needed */
}

void ShortFunction(void) {
    if (condition1) {
        ProcessCondition1();
    }
}

Rule: Code tells you HOW, comments tell you WHY (rationale, safety requirements, non-obvious algorithms)


Principle 5: Error Handling

Defensive Programming for Safety

ASPICE Requirement:

  • SWE.3 BP3: "Define interfaces of software units"
  • SWE.4 BP2: "Verify that interfaces comply with specifications"

Example: CAN Communication

Bad Error Handling (silent failure):

float ReadRadarDistance(void) {
    uint8_t buffer[8];
    CAN_ReadMessage(0x200, buffer);  /* What if this fails? */
    uint16_t raw = (buffer[0] << 8) | buffer[1];
    return raw / 1000.0;
}

[FAIL] Problems:

  • No error checking (CAN might timeout, return garbage)
  • Silent failure (returns wrong value, system doesn't know)

Good Error Handling (explicit):

/**
 * @brief Read obstacle distance from radar sensor via CAN
 * @param[out] distance_m Output: Distance in meters
 * @return 0 = success, -1 = CAN timeout, -2 = invalid data
 */
int ReadRadarDistance(float* distance_m) {
    uint8_t buffer[8];
    int status = CAN_ReadMessage(0x200, buffer, CAN_TIMEOUT_MS);

    /* Check for CAN timeout */
    if (status != CAN_OK) {
        Log_Error(ERROR_CAN_TIMEOUT, 0x200);
        return -1;  /* CAN timeout */
    }

    /* Extract distance from CAN message */
    uint16_t raw_distance = (buffer[0] << 8) | buffer[1];

    /* Validate range: Radar valid range 0-200 meters */
    if (raw_distance > 200000) {  /* 200m = 200,000mm */
        Log_Error(ERROR_RADAR_OUT_OF_RANGE, raw_distance);
        return -2;  /* Invalid data */
    }

    /* Convert mm to meters */
    *distance_m = raw_distance / 1000.0F;
    return 0;  /* Success */
}

/* Caller checks return code */
void ACC_Update(void) {
    float distance;
    int status = ReadRadarDistance(&distance);

    if (status != 0) {
        /* Handle error: Transition to safe state */
        ACC_Disable();
        return;
    }

    /* Normal operation */
    float target_speed = CalculateTargetSpeed(distance);
    ApplyControl(target_speed);
}

[PASS] Benefits:

  • Explicit error codes (caller knows what failed)
  • Error logging (diagnostics can trace root cause)
  • Fail-safe behavior (disable ACC on sensor failure)

Error Code Patterns

Pattern 1: Return Code + Output Parameter:

int GetData(DataType* output);  /* Returns 0 = OK, negative = error */

Pattern 2: Return Struct with Status:

typedef struct {
    float value;
    int status;  /* 0 = OK, negative = error */
} Result_t;

Result_t GetData(void);

Pattern 3: Error Enum (MISRA C:2012 compliant):

typedef enum {
    STATUS_OK = 0,
    STATUS_CAN_TIMEOUT = -1,
    STATUS_INVALID_DATA = -2,
    STATUS_SENSOR_FAULT = -3
} Status_t;

Status_t GetData(float* output);

Principle 6: Avoid Magic Numbers

Use Named Constants

Bad (magic numbers):

if (speed > 150) {           /* What is 150? */
    speed = 150;
}

if (distance < 10) {         /* What is 10? Meters? Feet? */
    Brake();
}

Good (named constants):

const int MAX_SPEED_KMH = 150;        /* ISO 26262 requirement */
const float MIN_SAFE_DISTANCE_M = 10.0;  /* 2-second rule at 20 km/h */

if (speed_kmh > MAX_SPEED_KMH) {
    speed_kmh = MAX_SPEED_KMH;
}

if (distance_m < MIN_SAFE_DISTANCE_M) {
    Brake_Apply();
}

MISRA C:2012 Rule 2.5: "A project should not contain unused macro declarations"

  • Define constants in header file, use consistently

Summary

Clean Code Principles for Embedded C:

  1. Meaningful Names: Self-documenting variables and functions (include units, avoid abbreviations)
  2. Small Functions: 5-15 lines, single responsibility, consistent abstraction level
  3. Do One Thing: One responsibility per function, separate concerns
  4. Comments Sparingly: Doxygen headers, traceability, safety rationale (not redundant, misleading, or journal comments)
  5. Error Handling: Explicit return codes, validate inputs, log errors, fail-safe behavior
  6. Avoid Magic Numbers: Use named constants with clear names and units

MISRA C:2012 Compliance: Clean code naturally aligns with MISRA rules (clear names, error handling, no magic numbers)

Next: Test-Driven Development (34.02) — Writing tests first to ensure correctness and 100% coverage


Self-Assessment Quiz

Test your understanding of clean code principles. Answers are at the bottom.

Question 1: Which function name best follows clean code principles for embedded C?

  • A) calc()
  • B) calculate_speed()
  • C) CalculateVehicleSpeed_kmh()
  • D) speed_calc_func_v2()

Question 2: What is the recommended line count for a function in clean code?

  • A) As many as needed to complete the task
  • B) 5-15 lines (one screen, single responsibility)
  • C) Exactly 20 lines
  • D) Less than 100 lines

Question 3: Which comment type should you keep in production code?

  • A) i++; // Increment i
  • B) /* 2023-05-12: Fixed bug in calculation (John) */
  • C) /* ISO 26262: Fail-safe to 0 km/h on sensor error */
  • D) } /* end for */

Question 4: What's wrong with this code: if (speed > 150) { speed = 150; }?

  • A) The comparison operator is wrong
  • B) Magic number 150 should be a named constant (MAX_SPEED_KMH)
  • C) Should use else clause
  • D) Nothing is wrong

Question 5: Which error handling pattern is correct for safety-critical code?

  • A) Silent failure: return default value if error occurs
  • B) Crash immediately on any error
  • C) Explicit return codes, error logging, and fail-safe transition
  • D) Retry indefinitely until operation succeeds

Quiz Answers

  1. C - Includes full context (Vehicle), action (Calculate), and units (kmh). PascalCase follows AUTOSAR conventions.

  2. B - 5-15 lines ensures single responsibility, easy testing, and fits on one screen for readability.

  3. C - Safety rationale explains WHY (ISO 26262 compliance). Options A, B, D are redundant, journal-style, or indicate overly long functions.

  4. B - Magic numbers (150) should be named constants (MAX_SPEED_KMH) for clarity and maintainability per MISRA Rule 2.5.

  5. C - Safety-critical code requires explicit error handling, logging for diagnostics, and defined fail-safe behavior.

Score Interpretation:

  • 5/5: Excellent clean code understanding
  • 3-4/5: Good foundation, review the specific principles for missed questions
  • 1-2/5: Re-read the chapter, practice with the refactoring exercises