I have implemented those two methods described as in 1
and 2
.
I appreciate any kind of review to improve my method implementation.
Implement the method
toString
.
toString
returns a string representation of the listImplement 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);
}
}
-
\$\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– default locale2017年03月28日 17:06:21 +00:00Commented Mar 28, 2017 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– Adam2017年03月28日 20:46:50 +00:00Commented Mar 28, 2017 at 20:46
2 Answers 2
Your toString()
method iterates up to courses.length
rather than courseCount
, so you will end up outputting null
s 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;
}
}
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.