1
\$\begingroup\$

I have implemented those two methods described as in 1 and 2. I appreciate any kind of review to improve my method implementation.

  1. Implement the method toString.
    toString returns a string representation of the list

  2. Implement the method remove .
    remove removes a given course from the list. If the course is not in the list, the list is not updated.

When the code is executed, the following printout is generated:

<10, Algebra>

<20, Algorithms>

<30, Electronics>

<40, Physics>

<50, Programming>

After the implemented methods call.

<10, Algebra>

<20, Algorithms>

<50, Programming>

public class CourseList {

    private static final int CAPACITY = 5;
    // courses
    private Course[] courses;
    // the number of courses
    private int courseCount;

    public CourseList() {
        courses = new Course[CAPACITY];
        courseCount = 0;
    }

    // add adds a course to the list
    public void add(Course course) {
        courses[courseCount] = course;
        courseCount++;
    }
    public void remove (Course course) {
        int courseIndex = -1;
        for (int i = 1; i < courseCount; i++)
            if (courses[i].equals(course)) {
                courseIndex = i;
                break;
            }
        if (courseIndex != -1) {
            for (int i = courseIndex; i < courseCount - 1; i++)
                courses[i] = courses[i + 1];
            courses[courseCount - 1] = null;
        }
    }

    public String toString(){
        StringBuilder sb = new StringBuilder();
        for(int i = 0; i< courses.length; i++){
            sb.append(courses[i]);
            sb.append("\n");
        }
        return sb.toString();
    }

    public static void main(String[] args) {
        CourseList list = new CourseList();
        list.add(new Course(10, "Algebra"));
        list.add(new Course(20, "Algorithms"));
        list.add(new Course(30, "Electronics"));
        list.add(new Course(40, "Physics"));
        list.add(new Course(50, "Programming"));
        System.out.println(list);
        list.remove(new Course(40, "Physics"));
        list.remove(new Course(30, "Electronics"));
        System.out.println(list);

    }
}
\$\endgroup\$
  • \$\begingroup\$ Are you allowed to use built-in collection classes (e.g. List)? Do you need to support more than five courses in a list? If there are any other constraints like this, please edit them into your question. \$\endgroup\$ – default locale Mar 28 '17 at 17:06
  • \$\begingroup\$ @defaultlocale I think in ArrayList it would be easy but it was not allowed if you have suggestion in built-in collection please add it's fun to learn. \$\endgroup\$ – Adam Mar 28 '17 at 20:46
2
\$\begingroup\$

Your toString() method iterates up to courses.length rather than courseCount, so you will end up outputting nulls if the CourseList is not filled to capacity.

Just a minor note: most StringBuilder methods are designed to allow chaining, like this:

sb.append(courses[i]).append("\n");

In your remove() method, you fail to decrement courseCount, so a subsequent add() will result in an awkward null gap.

You should never omit the "optional" braces of a for loop. Someday, you will contribute to a coding accident, and it will be your fault. Not to mention, the missing braces make the code look ugly and hard to read.

The method would be clearer and more reusable if you broke out part of the code into an indexOf(Course) method (which could be public or private — your choice).

public void remove(Course course) {
    int courseIndex = this.indexOf(course);
    if (courseIndex >= 0) {
        for (int i = courseIndex; i < courseCount - 1; i++) {
            courses[i] = courses[i + 1];
        }
        courses[--courseCount] = null;
    }
}
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Starting at 1 when removing an element will skip the first element. Remove shouldn't create a new array if it's not going to do anything with the new array.

And if you're going to modify the code to correct the unnecessary new array, you should also fix the problem with you starting at index 1.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.