0
public static void main(String [] args){
Scanner input = new Scanner(System.in);
  System.out.println("Enter some numbers (all on one line, separated by spaces):");
  String line = input.nextLine();
  String[] numbers = line.split(" +");
  ArrayList<Integer> a = new ArrayList<Integer>();
  for(int i=0; i<numbers.length; i++)
      a.add(new Integer(numbers[i]));
  System.out.println("The numbers are stored in an ArrayList");
  System.out.println("The ArrayList is "+a);
System.out.print("\nEnter a number: ");
  int p = input.nextInt();
  System.out.println(removeNumber(a,p));
  System.out.println(removeNumber2(a,p));
 }

 public static <T> ArrayList<T> removeNumber(ArrayList<T> a, Integer e)
 {

ArrayList<T> b = new ArrayList<T>();
  for(int i = 0; i< a.size();i++)
    {
    if (a.get(i).equals(e))

       a.remove(e);

    }
  return a;

}

if ex.value = 4, I want to remove 4 from the arrayList. If my arraylist contains [5,12,4,16,4], I want to remove the first occurence of four from it, and save it to another arraylist. Don't want to use Iterators

jayd
  • 11
  • 6
  • 2
    "Don't want to use Iterators" : why ? – ToYonos Oct 09 '14 at 15:10
  • 1
    Don't use `==` to evaluate object equality. – azurefrog Oct 09 '14 at 15:12
  • i havent done iterators yet – jayd Oct 09 '14 at 15:13
  • Be very careful about using an index to step through the list when you are removing items from the list. If the list is [3,5,5,6] and you want to remove the 5's, then when `i=1`, you remove the `5` and the list becomes [3,5,6]. But then you increment `i` to 2 before you even check again, which means you miss one of the elements. – ajb Oct 09 '14 at 15:15
  • 2
    `a.remove(e)` won't work because `e` is an `EX6` and `a` is not an `ArrayList`. I assume you're getting an error on this line? There are two ways to use `remove` correctly, see the [javadoc](http://docs.oracle.com/javase/8/docs/api/java/util/ArrayList.html#remove-int-). – ajb Oct 09 '14 at 15:17
  • possible duplicate of [Efficient equivalent for removing elements while iterating the Collection](http://stackoverflow.com/questions/223918/efficient-equivalent-for-removing-elements-while-iterating-the-collection) – JustinKSU Oct 09 '14 at 15:17
  • Actually, this isn't a compile-time error because `remove` takes an `Object`. It just won't work. – ajb Oct 09 '14 at 15:25

4 Answers4

0

You cannot iterate over an ArrayList and modify it at the same time, without an Iterator

Do like this :

for(Iterator<T> i = a.iterator(); i.hasNext();)
{
    if (i.next().equals(e.value))
    {
        i.remove();
    }
}

BTW, your b ArrayList is useless.

ToYonos
  • 16,469
  • 2
  • 54
  • 70
0

Along with the suggestion of using iterators, you should not be using the operator == which checks for reference equality. This will always be false in your scenario. You want to use the equals method instead which checks for value equality.

if (a.get(i).equals((Integer)e.value)))
JamesB
  • 7,774
  • 2
  • 22
  • 21
  • I would say == checks for *reference* equality instead of *object* equality, which IMO is ambiguous – Joffrey Oct 09 '14 at 15:19
0

You don't need to iterate through the list, either with an iterator or an index. ArrayList has a remove method that searches for and removes an element, and returns true or false depending on whether it found an element to remove. So you can say

while (true) {
    boolean found = a.remove(e.value);
    if (!found) {
        break;
    }
}

or

boolean found;
do {
    found = a.remove(e.value);
} while(found);

or if you value compactness over readability,

do { } while(a.remove(e.value));

Note that a.remove(e), as you have in your original code, won't work at all. The ArrayList is an ArrayList of Integer, but e is an EX6. Thus, a.remove(e) won't find e in the list at all, since it isn't even the correct type. It should compile fine, since remove is defined to allow any Object as a parameter, but it will never find anything (since the equals method of Integer always returns false for a non-Integer).

ajb
  • 31,309
  • 3
  • 58
  • 84
  • Note that this is O(n^2) since we have to start over from the beginning of the list every time, to search for the value. For an exercise where the lists would be small, I think this is OK. If really large lists are involved, I wouldn't do things this way, but I also wouldn't use `remove(int index)` which has to shift elements over every time it removes something, making it O(n^2) again. A different algorithm is needed. – ajb Oct 09 '14 at 15:32
0

Without using an iterator, here's what you could do to fix your code :

    for(int i = 0; i< a.size();i++) {
        if (a.get(i).equals(e.value)) {
            a.remove(e.value);
            i--;
        }
    }

Beyond the change of == to equals, you have to decrement i whenever you remove an element from the ArrayList. The reason for that is that removing the ith element from an ArrayList decreases the indices of all the elements that follow it by one. Therefore, the i+1th element will become the new ith element, so you must decrement i in order not to skip the next element.

EDIT : For some reason I was sure you wanted to remove all occurences of the number from the list, and not just the first one. If you only want to remove one element from the list, you don't have to worry about iterating over the rest of the list after removing that element.

Eran
  • 387,369
  • 54
  • 702
  • 768
  • Also, I personally disapprove of modifying an index in the body of a `for` loop, but that's a style issue. I think it makes things harder to follow. In a short loop like this, it's not a huge problem, but I'd avoid it and use `while`. – ajb Oct 09 '14 at 16:31
  • @ajb Oops, I totally missed that. That's what happens when you copy and paste code from the question and fix only the things you expect to be wrong. – Eran Oct 09 '14 at 16:31