Do the objects in this code follow OO standards?

Question

I previously posted my first attempt at writing a program that I was given to test my skills for a position that I was interviewing for. I have rewritten the program using the excellent advice that I received from that posting and I also started reading a book on Java Objects.

I would like to see if I am getting a better understanding of OO design with this version.

If anyone has suggestions for some good object design books, I would appreciate if you could post those too!

I didn't include the two exception classes, but they just extend Exception.

Thanks!

Sample input: "CSE258: CSE244 CSE243 INTR100", "CSE221: CSE254 INTR100", "CSE254: CSE111 MATH210 INTR100", "CSE244: CSE243 MATH210 INTR100", "MATH210: INTR100", "CSE101: INTR100", "CSE111: INTR100", "ECE201: CSE111 INTR100", "ECE111: INTR100", "CSE243: CSE254", "INTR100:"

public class CourseLoad {
    private List courseCatalog;

    public CourseLoad(String[] courseDescriptions) throws InvalidCourseNameException, InvalidCourseDescriptionException {
        this.courseCatalog = new ArrayList();
        for (int i = 0; i < courseDescriptions.length; i++) {
            courseCatalog.add(new Course(courseDescriptions[i], false));
        }
    }

    public List getCourseCatalog() {
        return courseCatalog;
    }
}


public class Schedule {
    private List courses;

    public Schedule() {
        courses = new ArrayList();
    }

    public boolean hasCourseBeenTaken(Course course) {
        return courses.contains(course);    
    }

    public void addCourse(Course course) {
        courses.add(course);
    }

    public String[] getPrintableSchedule() {
        String[] classes = new String[courses.size()];
        for (int i = 0; i < classes.length; i++) {
            classes[i] = ((Course) courses.get(i)).getCourseName();
        }
        return classes;
    }

    public List getCourses() {
        return courses;
    }
}


public class Course {
    private List prerequisites;
    private String department;
    private int courseNumber;
    private static final int COURSE_NUMBER_LENGTH = 3;

    public Course(String courseDescription, boolean isPrerequisite) throws InvalidCourseDescriptionException, InvalidCourseNameException {
        if (!isPrerequisite) {
            if (isValidCourseDescription(courseDescription)) {
                int colonIndex = courseDescription.indexOf(':');

                String courseName = courseDescription.substring(0, colonIndex);

                if (isValidCourseName(courseName)) {
                    department = courseName.substring(0, courseName.length() - COURSE_NUMBER_LENGTH);
                    courseNumber = Integer.parseInt(courseName.substring(courseName.length() - COURSE_NUMBER_LENGTH));
                    String prerequisites = courseDescription.substring(colonIndex);

                    if (prerequisites.length() > 1) {
                        this.prerequisites = new ArrayList();
                        prerequisites = prerequisites.substring(1);
                        StringTokenizer tokenizer = new StringTokenizer(prerequisites, " ");
                        while (tokenizer.hasMoreTokens()) {
                            String prerequisite = tokenizer.nextToken();
                            if (isValidCourseName(prerequisite)) {
                                this.prerequisites.add(new Course(prerequisite, true));
                            } else {
                                throw new InvalidCourseNameException("Invalid course name: " + courseName);
                            }
                        }
                    }
                } else {
                    throw new InvalidCourseNameException("Invalid course name: " + courseName);
                }
            } else {
                throw new InvalidCourseDescriptionException("Invalid course description: " + courseDescription);
            }
        } else {
            if (isValidCourseName(courseDescription)) {
                department = courseDescription.substring(0, courseDescription.length() - COURSE_NUMBER_LENGTH);
                courseNumber = Integer.parseInt(courseDescription.substring(courseDescription.length() - COURSE_NUMBER_LENGTH));
            } else {
                throw new InvalidCourseNameException("Invalid course name: " + courseDescription);
            }
        }
    }

    public String getCourseName() {
        return department + courseNumber;
    }

    public List getPrerequisites() {
        return prerequisites;
    }

    public String getDepartment() {
        return department;
    }

    public int getCourseNumber() {
        return courseNumber;
    }

    public boolean hasPrerequisites() {
        return prerequisites != null && prerequisites.size() > 0;
    }

    private boolean isValidCourseName(String courseName) {
        // validate the course name - i.e. "CSE111" or "MATH999"
        Pattern courseNamePattern = Pattern.compile("^[A-Z]{3,4}[1-9][0-9]{2}$");
        Matcher matcher = courseNamePattern.matcher(courseName);

        return matcher.matches();
    }

    private boolean isValidCourseDescription(String courseDescription) {
        // validate the course description - i.e. "CSE111: CSE110 MATH101"
        // or "CSE110:"
        Pattern courseDescriptionPattern = Pattern.compile("^[A-Z]{3,4}[1-9][0-9]{2}(:$|:(\\s[A-Z]{3,4}[1-9][0-9]{2})+)");
        Matcher matcher = courseDescriptionPattern.matcher(courseDescription);

        return matcher.matches();
    }

    public boolean equals(Object that) {
        if (this == that) return true;

        if ( !(that instanceof Course) ) return false;

        return ((Course) that).department.equals(this.department) &&
                ((Course) that).courseNumber == this.courseNumber;
    }

    public int hashCode() {
        return this.department.hashCode() + this.courseNumber;
    }
}


public class CourseScheduler {

    public CourseScheduler() {
    }

    public String[] scheduleCourses(String[] courseDescriptions) {
        CourseLoad courseLoad;
        Schedule schedule = new Schedule();

        if (courseDescriptions != null) {
            try {
                courseLoad = new CourseLoad(courseDescriptions);
                buildSchedule(schedule, courseLoad);
            } catch (InvalidCourseNameException e) {
                System.out.println(e.getMessage());
                schedule.getCourses().clear();
            } catch (InvalidCourseDescriptionException e) {
                System.out.println(e.getMessage());
                schedule.getCourses().clear();
            }
        }

        return schedule.getPrintableSchedule();
    }

    private void buildSchedule(Schedule schedule, CourseLoad courseLoad) throws InvalidCourseNameException {
        Course course;
        Course courseToAdd = null;

        while (courseLoad.getCourseCatalog().size() > 0) {
            for (int i = 0; i < courseLoad.getCourseCatalog().size(); i++) {
                course = (Course) courseLoad.getCourseCatalog().get(i);

                if (havePrerequisitesBeenTaken(course, schedule)) {
                    if (courseToAdd == null) {
                        courseToAdd = course;
                    } else if (course.getCourseNumber() < courseToAdd.getCourseNumber()) {
                        courseToAdd = course;
                    } else if (course.getCourseNumber() == courseToAdd.getCourseNumber()) {
                        if (course.getDepartment().compareTo(courseToAdd.getDepartment()) < 0) {
                            courseToAdd = course;
                        }
                    }
                }
            }

            if (courseToAdd == null) {
                throw new InvalidCourseNameException("No course found to add to schedule.");
            }

            schedule.addCourse(courseToAdd);
            courseLoad.getCourseCatalog().remove(courseToAdd);
            courseToAdd = null;
        }
    }

    private boolean havePrerequisitesBeenTaken(Course course, Schedule schedule) {
        if (!course.hasPrerequisites()) return true;

        List prerequisites = course.getPrerequisites();

        for (int i = 0; i < prerequisites.size(); i++) {
            if (!schedule.hasCourseBeenTaken((Course) prerequisites.get(i))) {
                return false;
            }
        }
        return true;
    }
}

Answer 1

Some quick comments:

For the getPrintableSchedule method in class Schedule. You can override the method toString() in the class Course, so you get a "printable" version of a Course.

In your for loops, you are calculating the length of your arrays in every iteration. Since you are not using threads and there is no risk of changing the size of your array during the execution of your loop, it is more efficient to extract it from your loop. Example: class Schedule - getPrintableSchedule

int arrayLength = classes.length;
for (int i=0; i < arrayLenght; i++) { ... }

In your constructor Course, there is some repeated code for getting the course department and number. You can create a method

public String getCourseDepartment(String myCourse) {
    return myCourse.substring(0, myCourse.lenght() - COURSE_NUMBER_LENGTH);
}

and call it with the appropiate String from the two points of your code.

Same thing for getting the course number.

It is more, I would refactor the constructor to something like this:

public Course(String courseDescription, boolean isPrerequisite) throws InvalidCourseDescriptionException, InvalidCourseNameException {
    if (isValidCourseDescription(courseDescription)) {
        String courseName = courseDescription;
        if (!isPrerequisite) {
            int colonIndex = courseDescription.indexOf(':');
            courseName = courseDescription.substring(0, colonIndex);
        }

        if (isValidCourseName(courseName)) {
            department = getCourseDepartment();
            courseNumber = getCourseNumber();
        }    
        ....
        // Process prerequisites, etc...
    }
}

• I think that some validations could be done twice as well. Does not course description validation ensure that the course name is valid?

Answer 2

wow that's alot of code in the Course constructor. Could this perhaps be factored out into some sort of load method. Or if you need to run this code for every course object, perhaps make a course createable using a static factory. So going with the refactoring suggestions already mentioned, you could do something like:

public class Course {

   protected Course() {
      // any initialisations could be done here as long as there are no side effects?
   }

   public static Course Create(String courseDescription, boolean isPrerequisite) throws InvalidCourseDescriptionException, InvalidCourseNameException {

      Course course = new Course(courseDescription);

      if(isPrerequisite) {
         course.LoadPrequisite(courseDescription);
      } else {
         course.Load(courseDescription);
      }

      return course();
   }

   protected void LoadPrerequisite(String courseDescription) {

   }

   protected void LoadCourse(String courseDescription) {

   }
}

then it would be used:

Course course = Course.Create(description, isPrequisite);

Answer 3

I think by far the most important advice I could give you is not to do any work in your constructors. Look up dependency injection. It basically states that you should ask for the objects that you need directly, instead of asking for building blocks and using those to construct your dependencies yourself.

So a simplified example:

//The wrong way, doesn't ask for what it needs, lies about its dependencies.
public Foo(Bar bar) {
    this.fooBar = bar.getFooBar();
}

//The right way, asks only for what it needs.
public Foo(FooBar fooBar) {
    this.fooBar = fooBar;
}

I recommend you watch this lecture by Misko Hevery, who explains this very well.

On an unrelated note: even if your constructor was a regular method, it would still be waaay too big.