Review and Refactor

public class CourseRoster {
    private String courseName;
    private Integer maxSize;
    private List<Student> enrolledStudents = new List<Student>();

    public CourseRoster(String courseName, Integer maxSize) {
        this.courseName = courseName;
        this.maxSize = maxSize;
    }

    public Boolean enroll(Student student) {
        if (student == null) {
            return false;
        }
        for (Student existing : enrolledStudents) {
            if (existing.getStudentId() == student.getStudentId()) {
                return false;
            }
        }
        if (enrolledStudents.size() >= maxSize) {
            return false;
        }
        enrolledStudents.add(student);
        return true;
    }

    public Integer getRemainingSeats() {
        return maxSize - enrolledStudents.size();
    }
}

This works correctly — but the enroll() method is doing three separate jobs (null-check, duplicate-check, capacity-check) inline. That's a candidate for refactoring.

public class CourseRoster {
    private String courseName;
    private Integer maxSize;
    private List<Student> enrolledStudents = new List<Student>();

    public CourseRoster(String courseName, Integer maxSize) {
        this.courseName = courseName;
        this.maxSize = maxSize;
    }

    public Boolean enroll(Student student) {
        if (student == null || isAlreadyEnrolled(student) || isFull()) {
            return false;
        }
        enrolledStudents.add(student);
        return true;
    }

    private Boolean isAlreadyEnrolled(Student student) {
        for (Student existing : enrolledStudents) {
            if (existing.getStudentId() == student.getStudentId()) {
                return true;
            }
        }
        return false;
    }

    private Boolean isFull() {
        return enrolledStudents.size() >= maxSize;
    }

    public Integer getRemainingSeats() {
        return maxSize - enrolledStudents.size();
    }
}

enroll() now reads almost like a sentence: reject if null, already enrolled, or full — otherwise add. isAlreadyEnrolled() and isFull() are private, since they're internal implementation details, not part of the class's public contract. External behavior hasn't changed at all — every existing call to enroll() behaves identically.

Getting something working first, then reviewing it with fresh eyes, is the normal shape of real development — not a sign the first version was wrong. A method with three unrelated checks jammed together isn't a bug, but it is harder to read, test, and extend than three named, focused pieces. Refactoring after it works, rather than trying to write the "perfect" version first, keeps each step small and verifiable.