I am trying to make an removeItem method to remove items of String from ArrayList. The item gets removed, but I still get a NullPointerException, not really sure why.
/**
* remove item
*/
public void removeItem(String description)
{
Iterator<Item> i = items.iterator();
Item items = i.next();
if items.getDescription().equals(description) {
i.remove();
}
else {
System.out.println("Invalid item description");
}
System.out.println("Item removed!");
}
Read this How do I compare strings in Java?
You should use .equals to check if two strings are equals.
And, when you are using an iterator you should check if any item is avaitable using hasNext method of the iterator.
There are anyway other methods like remove anyway!
Your code will be something like:
public void removeItem(String description)
{
Iterator<Item> i = items.iterator();
while (i.hasNext())
{
Item items = i.next();
if (items.getDescription().equals(description))
{
i.remove();
System.out.print("Item removed!");
return;
}
}
}
return because you should stop the while and exit the method if the item has been removed.
You should check if items and getDescription are not null!
With your code you will get a
"Invalid item description"
everytime anyway.
Try this - Using hasNext() method of Iterator & using null check
Iterator<Item> i = items.iterator();
while (i.hasNext()) {
Item items = i.next();
if (items.getDescription() != null && items.getDescription().equals(description)) {
i.remove();
System.out.print("Item removed!");
return;
}
}
This has a number of problems. See the comments I added to your code.
/**
* remove item
*/
public void removeItem(String description)
{
Iterator<Item> i = items.iterator(); //ok, so the variable items is probably a class field
Item items = i.next(); //...we just overwrote our class field items with the first item from iter...
if items.getDescription().equals(description) { //we don't have parenthesis around our if statement...?
i.remove(); //ok, we removed the item because it was equal
}
else {
System.out.println("Invalid item description"); //it wasn't equal, so we say we got an invalid description
}
//aaaand apparently we are done. We only checked the first item in "items"
System.out.println("Item removed!"); //this is outside of the if statement, so we will print this every time
}
So, let's clean up these problems. It seems like there could be multiple items with the same description, so let's change the method to reflect that as well.
/**
* removes all the items with a matching description
* returns the number of items removed
*/
public int removeItemsByDescription(String description) //make the method name reflect what it does
{
//do we want to be able to check against a null description? because this changes if that is the case
if(description == null)
throw new NullPointerException("description was null");
if(items == null) //make sure items is not null
throw new NullPointerException("Items was null");
Iterator<Item> itemsIter = items.iterator(); //more descriptive iterator name
Item item; //ok, we name our single item "item" - and we just declare it here, we'll assign it later
int removedItemNum = 0; //keep track of the total number of items we are removing
while(itemsIter.hasNext()){//let's go through all the items
item = = itemsIter.next(); //get the next item
if(description.equals(item.getDescription())){ //in order to avoid getting a null pointer exception when the description is null (which is probably perfectly valid), we will use description's .equals method
itemsIter.remove(); //remove the item
removedItemNum++; //increment the counter
}
//we don't need an else; if it didn't match, we just go on to the next one
}
System.out.println("Removed "+removedItemNum+" items");//print out how many we removed
}
And there we go! You should probably move that final print statement out of it when you are done testing.
Like I mentioned in the comments, this method changes if you want to be able to remove items with null descriptions (i.e., you want to pass in null as a valid parameter).
