Lab 01 — Checkstyle Enforcement

Phase: 05 — Code Quality and Engineering Discipline
Difficulty: ⭐⭐☆☆☆ | Estimated Time: 1.5–2 hours
Primary Artifact: FlawedScheduler.java with 5 planted Checkstyle violations
Verify: mvn checkstyle:check reports 0 violations after your fixes

Goal

FlawedScheduler.java is a task scheduler that compiles and runs correctly — but it has 5 Checkstyle violations that would be rejected by Apache CI. Your job is to:

1. Run mvn checkstyle:check and read the violation report
2. Identify each violation from the report (rule name, line number, description)
3. Fix each one so the build goes green
4. Understand why each rule exists

Setup

cd phase-05-code-quality/lab-01-checkstyle
mvn checkstyle:check

The build will fail immediately. Read the output — it lists each violation with:

• The rule name (e.g., ConstantName)
• The file and line number
• A short description of the violation

The 5 Violations

Find and fix each one. Do not look at the answers below until you have found them yourself in the report.

Steps

Step 1 — Baseline: read the report

mvn checkstyle:check 2>&1 | grep '\[ERROR\]'

Count the violations. There should be exactly 5. If you see more or fewer, check that you haven't already modified the file.

Step 2 — Fix one violation at a time

Fix them in this order (each fix is isolated):

1. Unused import — easiest, just delete the line
2. Constant name — rename field + update all references in the same file
3. Magic number — extract to a constant above the field declarations
4. Missing Javadoc — add a complete Javadoc block
5. Line too long — restructure the LOG.warning(...) call

After each fix, run mvn checkstyle:check to confirm the violation count decreases by 1.

Step 3 — Green build

mvn checkstyle:check && echo "ALL CLEAN"

This must print ALL CLEAN.

Step 4 — Confirm the class still compiles and the scheduler still works

mvn test

There are no unit tests in this module (the scheduler is tested manually), but the build must succeed with no compilation errors.

Step 5 — Reflection

Answer these questions in a comment at the top of your fixed FlawedScheduler.java:

1. What would happen if you left maxRetries as a magic number 3 in the sleep condition?
2. Why does Checkstyle flag ArrayList when it compiles fine without being used?
3. What's the downside of the Checkstyle LineLength rule being set to 80 instead of 100?

Verification

# Zero Checkstyle violations
mvn checkstyle:check
echo "Exit code: $?"   # must be 0

# Code still compiles
mvn compile

Source Files

checkstyle.xml

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN" "https://checkstyle.org/dtds/configuration_1_3.dtd">
<!--
  Checkstyle configuration for Lab 01 — Code Quality.

  This file deliberately enables exactly the rules needed to catch the 5
  violations planted in FlawedScheduler.java. It is a subset of what a real
  Apache project (e.g., Spark, Kafka) would enforce.

  To generate a report without failing the build:
    mvn checkstyle:checkstyle
    open target/reports/checkstyle.html

  To fail the build on violations (CI mode):
    mvn checkstyle:check
-->
<module name="Checker">
    <property name="charset" value="UTF-8" />
    <!--
    severity=error means violations are reported as errors (not warnings)
    and will fail the build when maven-checkstyle-plugin's failsOnError=true.
  -->
    <property name="severity" value="error" />

    <!--
    Rule 5: LineLength
    Maximum line length is 100 characters.
    Placed under Checker (not TreeWalker) because it is a file-level check
    that doesn't require AST parsing.

    Why 100 and not 80? Modern monitors are wide enough that 100 columns
    avoids unnecessary line wrapping without going so far that lines are
    unreadable on a split-screen laptop.
  -->
    <module name="LineLength">
        <property name="max" value="100" />
        <!-- Ignore lines that are only a URL (can't be wrapped sensibly) -->
        <property name="ignorePattern" value="^ *\* *https?://" />
    </module>

    <module name="TreeWalker">

        <!--
      Rule 1: UnusedImports
      Flags any import statement whose type is never referenced in the file.

      Why it matters: unused imports are a readability smell — they suggest the
      code was copy-pasted or a refactor left orphaned imports behind. They also
      cause spurious merge conflicts in files that many contributors touch.
    -->
        <module name="UnusedImports" />

        <!--
      Rule 2: ConstantName
      static final fields (constants) must match UPPER_SNAKE_CASE.

      Default pattern: ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$
      Violating example: static final int maxRetries = 3;
      Correct example:   static final int MAX_RETRIES = 3;

      Why it matters: UPPER_SNAKE_CASE is the universal Java constant convention.
      A lowercase name for a static final field looks like a mutable field at a
      glance, making it harder to reason about mutability.
    -->
        <module name="ConstantName" />

        <!--
      Rule 3: MagicNumber
      Numeric literals other than the listed "ignoreNumbers" are flagged.

      Ignored by default: -1, 0, 1, 2  (universal, self-documenting)
      We also ignore 100 because it appears in the queue capacity constructor
      and is self-documenting in context.

      Violating example: Thread.sleep(5000)
      Correct example:   Thread.sleep(RETRY_DELAY_MS)

      Why it matters: a magic number tells you the value but not the intent.
      RETRY_DELAY_MS is searchable, changeable in one place, and self-documenting.
    -->
        <module name="MagicNumber">
            <property name="ignoreNumbers" value="-1, 0, 1, 2, 100" />
            <property name="ignoreAnnotation" value="true" />
        </module>

        <!--
      Rule 4: MissingJavadocMethod
      Public methods must have a Javadoc comment (/** ... */).
      Scope=public means package-private, protected, and private methods
      are NOT required to have Javadoc (though it is encouraged).

      Why it matters: public methods are part of the library's API contract.
      Without Javadoc, callers don't know what the method promises, what
      null behavior looks like, or what exceptions can be thrown.
    -->
        <module name="MissingJavadocMethod">
            <property name="scope" value="public" />
        </module>

    </module>
</module>

pom.xml

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>org.example.quality</groupId>
    <artifactId>checkstyle-lab</artifactId>
    <version>1.0.0-SNAPSHOT</version>
    <packaging>jar</packaging>
    <name>Code Quality Lab 01 — Checkstyle</name>

    <properties>
        <maven.compiler.source>11</maven.compiler.source>
        <maven.compiler.target>11</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    </properties>

    <build>
        <plugins>
            <!--
        maven-checkstyle-plugin: runs Checkstyle against the source files.

        configLocation: path to checkstyle.xml, relative to the project root.
        failsOnError: if true, the build fails when violations are found.
        violationSeverity: minimum severity level that counts as a violation
          (must match <property name="severity" value="error"/> in checkstyle.xml).
        includeTestSourceDirectory: also check test sources (set false here to
          focus only on main sources for this lab).
        consoleOutput: print violations to the console (not just the report file).

        Run:  mvn checkstyle:check   → fail on violations (CI mode)
              mvn checkstyle:checkstyle → generate report only (no build failure)
      -->
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-checkstyle-plugin</artifactId>
                <version>3.3.1</version>
                <configuration>
                    <configLocation>checkstyle.xml</configLocation>
                    <failsOnError>true</failsOnError>
                    <violationSeverity>error</violationSeverity>
                    <includeTestSourceDirectory>false</includeTestSourceDirectory>
                    <consoleOutput>true</consoleOutput>
                </configuration>
                <!--
          Bind checkstyle:check to the 'verify' lifecycle phase so that
          running `mvn verify` automatically checks style.
          This is how Apache projects integrate Checkstyle into CI.
        -->
                <executions>
                    <execution>
                        <id>checkstyle-verify</id>
                        <phase>verify</phase>
                        <goals>
                            <goal>check</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>
        </plugins>
    </build>

</project>

FlawedScheduler.java (before your fixes)

package org.example.quality;

// VIOLATION 1 — UnusedImports
// ArrayList is imported but never used in this file.
// Fix: delete this import.
import java.util.ArrayList;

import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.logging.Logger;

/**
 * A simple retry-capable task scheduler backed by a bounded blocking queue.
 *
 * <p>
 * Tasks submitted via {@link #submit(Runnable)} are queued and executed
 * in order by {@link #runAll()}. If a task throws, it is re-queued for up
 * to {@code MAX_RETRIES} attempts with a delay between each attempt.
 */
public class FlawedScheduler {

    private static final Logger LOG = Logger.getLogger(FlawedScheduler.class.getName());

    // VIOLATION 2 — ConstantName
    // Static final fields (constants) must use UPPER_SNAKE_CASE.
    // 'maxRetries' starts with a lowercase letter and uses camelCase.
    // Fix: rename to MAX_RETRIES and update all references in this file.
    static final int maxRetries = 3;

    private final String name;
    private final BlockingQueue<Runnable> queue;

    /**
     * Creates a scheduler with the given name and queue capacity.
     *
     * @param name     a human-readable label used in log messages
     * @param capacity maximum number of tasks that can be queued at once
     */
    public FlawedScheduler(String name, int capacity) {
        this.name = name;
        this.queue = new LinkedBlockingQueue<>(capacity);
    }

    // VIOLATION 3 — MissingJavadocMethod
    // This public method has no Javadoc comment.
    // Fix: add /** @param task the task to enqueue; ignored if null */ above.
    public void submit(Runnable task) {
        if (task != null) {
            queue.offer(task);
        }
    }

    /**
     * Drains the queue, executing each task in order.
     *
     * <p>
     * If a task throws an exception, it is re-queued and retried after a delay,
     * up to {@code MAX_RETRIES} times. Tasks that exceed the retry limit are
     * permanently dropped with a warning log.
     *
     * @throws InterruptedException if the thread is interrupted while sleeping
     *                              between retries
     */
    public void runAll() throws InterruptedException {
        int attempt = 0;
        while (!queue.isEmpty()) {
            Runnable task = queue.poll();
            if (task != null) {
                try {
                    task.run();
                    attempt = 0;
                } catch (Exception ex) {
                    attempt++;
                    if (attempt <= maxRetries) {
                        // VIOLATION 4 — MagicNumber
                        // 5000 is a magic number. Its meaning (milliseconds? retries?) is unclear.
                        // Fix: extract to a named constant, e.g.:
                        // private static final long RETRY_DELAY_MS = 5000L;
                        Thread.sleep(5000);
                        queue.offer(task); // re-enqueue for retry
                    } else {
                        // VIOLATION 5 — LineLength
                        // This line is longer than 100 characters (count from 'LOG' to the closing
                        // ')').
                        // Fix: split the string concatenation across multiple lines, or use a local
                        // variable.
                        LOG.warning("Task permanently failed after " + maxRetries + " retries on scheduler '" + name
                                + "': " + ex.getMessage());
                        attempt = 0;
                    }
                }
            }
        }
    }
}