Unable to remove elements from ArrayBag

[Krocker]

hi, so im working on a dice assignment where the users creates dice of 4, 6, 20 and 100 sided. Most of the program seems to work, but i cant seem to make the program remove the die when specified by the user. The program adds the die without any problems, but wont remove it. The Dice, and IntArrayBag classes was given to me by the prof, the only thing i had to do was modify the ArryBag to a generic type; maybe something went wrong there? I put a print statement to see where it was going wrong, but the method keeps returning false even though i know the die type is in the bag. What am i doing wrong?

[DemonWasp]

The problem is that the equality operator, ==, behaves differently between primitives (int, char, long, float,...) and object references. On objects, == will compare the references themselves, not the contents of the object.

You will have to call .equals() to determine which object to remove.

[Krocker]

DemonWasp @ Tue Apr 01, 2014 12:10 pm wrote:

The problem is that the equality operator, ==, behaves differently between primitives (int, char, long, float,...) and object references. On objects, == will compare the references themselves, not the contents of the object.

You will have to call .equals() to determine which object to remove.

ok, so i modifyied the intarraybag remove method, and changed the == to .equals, but its still unable to find the die. Was that wat you meant?

code:

public boolean remove(E target)    {       int index; // The location of target in the data array.               // First, set index to the location of target in the data array,       // which could be as small as 0 or as large as manyItems-1; If target       // is not in the array, then index will be set equal to manyItems;       for (index = 0; (index < manyItems) && !(target.equals(data[index])); index++) //Modified this part!          // No work is needed in the body of this for-loop.          ;                 if (index == manyItems)          // The target was not found, so nothing is removed.          return false;       else       {  // The target was found at data[index].          // So reduce manyItems by 1 and copy the last element onto data[index].          manyItems--;          data[index] = data[manyItems];          return true;       }    }

[DemonWasp]

Your change looks correct. You might also consider looking at your Remove ActionListener, because it looks like it only knows how to remove the 4-sided die.

[Krocker]

DemonWasp @ Tue Apr 01, 2014 12:50 pm wrote:

Your change looks correct. You might also consider looking at your Remove ActionListener, because it looks like it only knows how to remove the 4-sided die.

i only wrote the remove for the 4 sided die just to test, didnt want to write a bunch of code to find it doesnt work. Also, i found out that its actaully the variables i use for the dice.

Ok so i modified my program and managed to make it delete the die. However, the program will only delete one of each die only once. Im also having the same program with the occurrence method in the IntArrayBag. It will show the occurrence the 1st time it is added, but after that, it stays at 1 even if another of the same die is entered. I cant figure out why and its driving me nuts.

[Krocker]

can someone please help me? im having a very hard time trying to figure this out!

[DemonWasp]

Okay, I looked deeper and I've found a few problems.

The subtle bug that's causing your problem is that in Die.java you declared public boolean equals (Die in) but you should have declared public boolean equals ( Object in ). You then have to test whether "in" corresponds to a Die instance (using the instanceof operator) before casting it to a Die instance and doing the other equality checks (based on the number of sides).

The reason that you need the latter form is because then you override the Object.equals() implementation. Because of type erasure in generics (which could be considered a design problem in Java), the code in IntArrayBag ends up looking like:

Java:

private Object[] data; //... public boolean remove ( Object target ) {     // ...     target.equals(data[index])     // ... }

At the target-equals-data line, data[index] is an Object, so the method called is target's implementation of equals(Object). When you overload "equals" with the equals(Die) method, you are not overriding the equals(Object) method, so you get the inherited Object.equals(Object) implementation.

If you are using Java 5+, you can annotate methods with @Override to make sure the compiler warns you if you aren't actually overriding things when you think you are. This is especially helpful for equals() and hashcode().

You have a few other bugs, too:

1) When removing an object, a reference to the last element gets copied over the removed element's reference, but not removed from its original position. This means you'll hold on to objects longer than you should (they'll stay lurking in the array past the 'manyItems' limit--which is itself an awful name for 'size').

2) When the bag is empty, grab() will return the object that is kept around by bug #1.