CODEWITHSUNDEEP
javaarraylistiterator
Sevrok
Sevrok

Reputation: 3

Delete an object from ArrayList by iterator

I want to create a program which would be like a home budget, so I have a class AmountModel (I know that Integer is not so good for id, but it's not a problem now):

import java.time.LocalDate;

public class AmountModel {
  private Integer id;
  private Double amount;
  private CategoryModel categoryModel;
  private LocalDate localDate;

  // getters/setters etc.
}

And in another class I built this deleteAmount method: 

static Scanner sc = new Scanner(System.in);

public List<amountModel> deleteAmount() {
    Iterator<AmountModel> it = amountList.iterator();
    while (it.hasNext()) { 
        System.out.println("Choose index to delete ");
        AmountModel am = it.next();
        if (am.getId().equals(sc.nextInt())) {
            it.remove();
        }
        break;
    }
    return amountList;
}

Adding object works good, but when I try to use the delete method I have to put first index.

Example:
I have three objects (with index 0, 1, 2).

What is wrong with this method?

Upvotes: 0

Views: 108

Answers (4)

Jesse Stuart
Jesse Stuart

Reputation: 596

Welcome to Stack Overflow!

As others have mentioned, there are a few ways to tackle this. But I think you can make this even simpler by changing the data structure used to access your AmountModel collection: if you're frequently accessing an item by ID, a Map is a great fit.

No more worrying about iterator state; you could just do something like:

// Map "amounts" by ID for easy O(1) lookup.
static Map<Integer, AmountModel> amountMap

public void deleteAmount(Integer id) {
  if (!amountMap.containsKey(id)) { 
    // (TODO: Handle invalid input)
    throw new Exception()
  }

  amountMap.remove(id)
  return
}

Hope this helps! I threw together a working example in a gist here if you're interested. (In Groovy, but should be enough to give you the idea)

Upvotes: 1

user3197884
user3197884

Reputation: 26

Your break statement is breaking the while loop in the first iteration only. So, it will work only if the first am.getId() matches with your fist input. Also, your sc.nextInt() will keep on scanning for next available input, Remove it from while loop. 

static Scanner sc = new Scanner(System.in);
public List<AmoutModel> deleteAmount() {
    Iterator<AmoutModel> it = amountList.iterator();
    Integer scId = sc.nextInt();
    while (it.hasNext()) { 
        System.out.println("Choose index to delete ");
        AmoutModel am = it.next();
        if (am.getId().equals(scId)) {
            it.remove();
            break;
        }
    }
    return amountList;
}

Upvotes: 0

Ryan
Ryan

Reputation: 1760

You should separate your input logic from your delete logic and accept the list as a parameter.

Note: this only works with a mutable list. If you use something like Arrays.asList() it will throw an exception.

public void deleteAmount(List<AmountModel> list, int key) {
    list.removeIf(a -> a.getId().equals(key));
}

Upvotes: 1

Slackow
Slackow

Reputation: 312

call your sc.nextInt() outside of the loop, otherwise it will get run everytime the loop returns, as the condition gets reevaluated every time the loop ends. also you could use the remove method of list

    static Scanner sc = new Scanner(System.in);
    public List<AmoutModel> deleteAmount() {
        System.out.println("Choose index to delete ");
        int index = sc.nextInt();
        amountList.remove(index);
        return amountList;
    }

Upvotes: -1

Related Questions