1

this is my first time asking a question on here so I apologize for any mistakes in terms of form.

I am working on an assignment for AP Computer Science which involves generating a listarray filled with random ints (within a certain range) and then processing them with methods that remove objects which either exceed or are less than a threshold determined earlier in the program. Here is an example of the code I wrote along with the preconditions provided by my teacher.

/**
* @param orig is a List of Integer
* @param mid is an int > 2
* @return a new List of Integer that contains, in order, all the numbers in
* orig that are >= mid / 2
*/public static ArrayList<Integer> extractUpper(ArrayList<Integer> orig, int mid) {
    ArrayList<Integer> myArray = new ArrayList<Integer>();
    for(Integer a: orig) {
        if (a >= mid/2)
            myArray.add(a);
    }
    return myArray;
}

/**
* @param orig is a List of Integer
* @param mid is an int > 2
* @return none PostCondition: all numbers less than  mid / 2 have been 
*  removed from orig
*/
public static void deleteUpper(ArrayList<Integer> orig, int mid) {
    for(int j = 0; j < orig.size(); j++) {
        if (orig.get(j) >= (mid/2))
            orig.remove(j);
    }

}

To me, this seems like it should work fine, but when I run this:

ic static void main(String[] args) {
    //a.
    int listLen = 10;
    int listMax = 20;
    System.out.println("listLen equals " + listLen + " and listMax equals " + listMax);
    System.out.println();

    //b.
    System.out.println("Generating a fixed-length ArrayList of length " + listLen + " with all values >= 0 and < " + listMax);
    ArrayList<Integer> Array1 = Main.buildFixedList(listLen, listMax);
    System.out.println(Array1);

    //c.        
    System.out.print("The numbers in this ArrayList >= " + listMax/2 + " are: ");
    ArrayList<Integer> Array2 = Main.extractUpper(Array1, listMax);
    System.out.println(Array2);

    //d.
    System.out.print("After deleting numbers > " + listMax/2 + " the modified list is: ");
    Main.deleteUpper(Array1, listMax);
    System.out.println(Array1);

    //e.
    System.out.print("After deletion, the numbers in the List >= " + listMax/2 + " are: ");
    ArrayList<Integer> Array3 = Main.extractUpper(Array1, listMax);
    System.out.println(Array3);

    //f.
    System.out.println();

My output seems to ignore certain numbers, some more frequently than others.

listLen equals 10 and listMax equals 20

Generating a fixed-length ArrayList of length 10 with all values >= 0 and < 20
[14, 16, 12, 9, 8, 11, 14, 16, 1]
The numbers in this ArrayList >= 10 are: [14, 16, 12, 11, 14, 16]
After deleting numbers > 10 the modified list is: [16, 9, 8, 14, 1]
After deletion, the numbers in the List >= 10 are: [16, 14]

The >=10 and <10 methods work occasionally, but I figure it's more of a crap-shoot right now. In this particular example the >=10 method worked but the <10 did not. I am at a loss as to what is wrong with my code.

EDIT: Thank you for all the replies, I appreciate the help. I have edited both the extractUpper and deleteUpper methods and am getting an even higher rate of success, but the code just seems to ignore some numbers. Here's the code:

 /**
* @param orig is a List of Integer
* @param mid is an int > 2
* @return a new List of Integer that contains, in order, all the numbers in
* orig that are >= mid / 2
*/public static ArrayList<Integer> extractUpper(ArrayList<Integer> orig, int mid) {
    ArrayList<Integer> myArray = new ArrayList<Integer>();
    for (int i = 0;  i < orig.size(); i++){
        if(orig.get(i) >= mid/2) {
            myArray.add(orig.get(i));
        }
    }
    return myArray;
}

/**
* @param orig is a List of Integer
* @param mid is an int > 2
* @return none PostCondition: all numbers less than  mid / 2 have been 
*  removed from orig
*/
public static void deleteUpper(ArrayList<Integer> orig, int mid) {
    for ( int i = orig.size()-1;  i >= 0; i--){
        if (i < orig.size()) {
            if(orig.get(i) >= mid/2) {
                orig.remove(i);
                i++;
            }
        }
        else
            i--;
    }

}

Here are a few outputs directly from the program:

listLen equals 10 and listMax equals 20

Generating a fixed-length ArrayList of length 10 with all values >= 0 and < 20
[4, 15, 8, 11, 18, 16, 7, 3, 6]
The numbers in this ArrayList >= 10 are: [15, 11, 18, 16]
After deleting numbers > 10 the modified list is: [4, 8, 7, 3, 6]
After deletion, the numbers in the List >= 10 are: []

Generating a fixed-length ArrayList of length 10 with all values >= 0 and < 20
[6, 3, 9, 16, 4, 4, 17, 8, 4]
The numbers in this ArrayList >= 10 are: []
After deleting numbers > 10 the modified list is: [6, 3, 9, 4, 4, 8, 4]
After deletion, the numbers in the List >= 10 are: []

listLen equals 10 and listMax equals 20

Generating a fixed-length ArrayList of length 10 with all values >= 0 and < 20
[4, 5, 0, 4, 12, 12, 1, 12, 10]
The numbers in this ArrayList >= 10 are: [12, 12, 12, 10]
After deleting numbers > 10 the modified list is: [4, 5, 0, 4, 1, 12]
After deletion, the numbers in the List >= 10 are: [12]

Generating a fixed-length ArrayList of length 10 with all values >= 0 and < 20
[15, 16, 2, 8, 1, 7, 3, 0, 15]
The numbers in this ArrayList >= 10 are: [12]
After deleting numbers > 10 the modified list is: [2, 8, 1, 7, 3, 0]
After deletion, the numbers in the List >= 10 are: [12]
M. Gaines
  • 13
  • 4
  • 1
    I indicated a possible duplicate for this question, but with the wrong topic. Look at this one, it should help you : http://stackoverflow.com/questions/19461632/why-arraylist-remove-is-not-working – Arnaud Jan 28 '16 at 13:47
  • 1
    Use a debugger to step through your `deleteUpper()` method. Pay close attention to `j` and your ArrayList, particularly before and after calling `.remove()`. – dsh Jan 28 '16 at 13:51
  • One solution to your problem involves [this method](https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html#remove--) – dsh Jan 28 '16 at 13:52

2 Answers2

2

Your problem is in the function deleteUpper You should iterate in reverse order the collection to be able to remove item without impacting the original index of the collection

Christophe M
  • 21
  • 3
  • I have reversed the loop and it has helped a little bit. Outputs are right more often than they are wrong now, but it is not 100% – M. Gaines Jan 28 '16 at 14:17
0

The problem is in the deleteUpper method.

When you delete an item from an List, the indexes in that List change - if you remove item 3, the item that was previously accesible at index 4 now becomes number 3.

In your implementation you always increase the index pointer, regardless if the deletion happened or not. This means that if two consecutive items meet the deletion criterion, only the first one will be removed.

Use an Iterator instead:

Iterator<Integer> i = orig.iterator();
while (i.hasNext()) {
    if (i.next() >= mid) {
        i.remove();
    }
}

If you don't want to use an Iterator:

for (int i=0;  i<orig.size(); ) {
    if (orig.get(i) >= mid) {
        orig.remove(i);
    }
    else {
        i++;
    }
}
Grogi
  • 2,099
  • 17
  • 13
  • We have not covered iterators in our course yet, so there is definitely a way to go about this without using them. I have tried processing the array backwards, and stepping back in the loop once a change is made. It has improved results marginally. – M. Gaines Jan 28 '16 at 14:15
  • Certainly there is. You can iterate from the end - which I don't really like because of the messy continue condition (won't work with unsigned integers). What you have done above is even messier ;) Why not something like the example I've just added above? – Grogi Jan 28 '16 at 14:57
  • Thank you for your help, I have managed to solve the problem. The issue seemed to be a combination of the deleteUpper method and overwriting Array1 more times than I thought I was. – M. Gaines Jan 28 '16 at 16:21