<![CDATA[
"Bad" ArrayIntList class
// Stuart Reges (my name is Elroy Jetson)
// This is the ArrayIntList class.
public class ArrayIntList {
int[] elementData; // element data
int size; // size
int capacity; // capacity
public static int defaultCapacity = 100;
public ArrayIntList() {
elementData = new int[100];
size = 0;
capacity = 100;
}
// capacity should be not be negative
public ArrayIntList(int capacity) {
if (capacity < 0) {
throw new IllegalArgumentException();
} else {
elementData = new int[capacity];
size = 0;
this.capacity = capacity;
}
}
public int size() {
return size;
}
// pre : 0 <= index < size() (throws exception if not)
public int get(int index) {
if (index < 0 || index >= size) {
throw new IndexOutOfBoundsException();
}
for (int i = 0; i < size; i++) {
if (i == index) {
return elementData[i];
}
}
return 0;
}
public String toString() {
if(size==0) {
return"[]";
} else {
String result="["+elementData[0];
for(int i=1;i<size;i++) {
result+=", "+elementData[i];
}
return result+"]";
}
}
// uses a for loop to find out if the array has the given value
public boolean contains(int value) {
for (int i = size - 1; i >= 0; i--) {
if (elementData[i] == value) {
return true;
}
}
return false;
}
public boolean isEmpty() {
if (size == 0) {
return true;
} else {
return false;
}
}
// returns the position of the given value in the array, only checking up
// to the current size
public int indexOf(int value) {
int index = 0;
for (int i = size - 1; i >= 0; i--) {
if (elementData[i] == value) {
index = i;
}
}
if (elementData[index] == value) {
return index;
} else {
return -1;
}
}
// pre : size() < capacity (throws IllegalStateException if not)
// post: appends the value to the end of the list
public void add(int value) {
if (size > capacity - 1) {
throw new IllegalStateException();
}
elementData[size] = value;
size++;
}
// pre : size() < capacity (throws IllegalStateException if not) &&
// 0 <= index <= size() (throws IndexOutOfBoundsException if not)
// post: inserts the value at the index
public void add(int index, int value) {
if (index < 0 || index > size) {
throw new IndexOutOfBoundsException();
}
if (size > capacity - 1) {
throw new IllegalStateException();
}
for (int i = size; i > index; i--) {
elementData[i] = elementData[i - 1];
}
elementData[index] = value;
size++;
}
// pre : 0 <= index < size() (throws IndexOutOfBoundsException if not)
// post: removes value at the index and descreases the size by 1
public void remove(int index) {
if (index < 0 || index >= size) {
throw new IndexOutOfBoundsException();
}
for (int i = index; i < size - 1; i++) {
elementData[i] = elementData[i + 1];
}
size--;
}
}
]]>
<![CDATA[
Below is a list of style problems with the bad ArrayIntList:
class comment: Don't include Stuart's name, include just your name. It
would also be helpful to include the date and your section or TA's name.
The class comment is meaningless. Make some kind of attempt to describe
what the class is used for, as in "Class ArrayIntList can be used to store
a list of integers."
fields: Fields should be declared private. The field comments are useless
because they are repeating the names of the fields. Only include comments
if you can provide something beyond the field name. The field called
"capacity" is not needed because it has the same value as
elementData.length.
class constant: It is improperly declared because it is missing "final".
It is also improperly named because the convention for constants is to use
all uppercase letters and underscore characters, as in DEFAULT_CAPACITY.
first constructor: It should have a comment and it should be using the
"this(...)" notation to call the other constructor. It does not use the
class constant as it should.
second constructor: Comment doesn't mention the fact that it throws an
IllegalArgumentException when capacity is negative and the comment should
describe more about what it does. The use of if/else is not appropriate.
The convention in Java is to throw exceptions with an if and then to have
the standard code follow without being inside an else.
size method: It has no comment.
get method: Comment doesn't mention what kind of exception is thrown and
doesn't describe what it does. The for loop is not needed and makes the
method extremely inefficient, basically negating the benefit of using an
array (the random-access aspect of the array).
toString method: No comment. Spacing is terrible. Introduce spaces to
make your code more readable. The indentation is also off on many lines.
contains method: The comment has implementation details, discussing the use
of a for loop and the fact that it is searching an array. This method is
also highly redundant. It should call indexOf.
isEmpty method: No comment and it violates boolean zen.
indexOf method: The comment has implementation details, talking about the
array and the size fields. It also does not describe significant behavior:
the fact that the first occurrence is returned and that a -1 is returned if
not found. The implementation is horrible. It uses a variable called
index that is not needed and then there is a redundant test after the
loop. Even if you are going to use this index variable, then initialize it
to -1 and return it after the loop instead of having the same test both
inside and outside the loop.
first add method: This is a good method. You can eliminate some redundancy
by having it call the other add method, but that is a subtle point that we
wouldn't expect students to notice.
second add method: The exception comments are good, but the description of
what it does is incomplete. It doesn't say what happens to the old value
at the given index. It shifts subsequent values to the right, which should
be described in the comment so that the client knows what it does.
remove method: The comment shouldn't discuss the implementation detail of
decreasing the size and it should mention what happens to the list when the
value is removed. The subsequent values are shifted to the left.
overall: The lines of code to check for illegal indexes in get and remove
are redundant. It would have been good to introduce a private method to
eliminate the redundancy.
]]>