Greenfoot

fejfo

2015/2/22

#

I have looked up some explanations on the internet but they are all very difficult. I have a list of items and I'm trying to combine items with the same id to 1 item of that id and the some of the amounts.

ArrayList<Item> item = new ArrayList();

public void fuseItems() {
        for(Item reference:item) {
            for(Item checking:item) {
                if(reference.id == checking.id & reference != checking) {
                    reference.amount+=checking.amount;
                    item.remove(reference);
                }
            }
        }
    }

in Item :

import greenfoot.*;  // (World, Actor, GreenfootImage, Greenfoot and MouseInfo)

/**
 * Write a description of class Item here.
 * 
 * @author (your name) 
 * @version (a version number or a date)
 */
public class Item
{
    int amount;
    String id;
    public Item(String newID,int newAmount) 
    {
        id = newID;
        amount = newAmount;
    } 
    
    public Item(String newID) 
    {
        id = newID;
        amount = 1;
    } 
    
    public String toString() {
        return "\nItem: id: "+id+"\t amount: "+amount;
    }
}

danpost

2015/2/22

#

The problem is that in your 'fuseItems' method, at lines 4 and 5 above, you are iterating through the 'item' list. Inside the loop at line 8 you are changing the structure of that list by trying to remove an element from it. This is what concurrent modification is referring to. You are trying to modify the List object while using it in a way where its structure must stay intact (iterating though the list). EDIT: There was a flaw in my initial solution. Hopefully, I can have a solution forthcoming.

danpost

2015/2/22

#

I think the following will do what you want:

public void fuseItems() {
    ArrayList<Item> list = new ArrayList<Item>(item);
    while (! list.isEmpty()) {
        Item reference = list.remove(0);
        for (Item checking : list) {
            if (reference.id == checking.id) {
                reference.amount += checking.amount;
                item.remove(checking);
            }
        }
    }
}

It creates a copy of the 'item' list on line 2. Line 3 creates a loop that executes until the copied list is empty. Line 4 removes the first element in the copied list, holding it in a local variable reference. Since the element is removed, the list now contains only those elements that it needs to be compared to -- so, line 5 interates through the remainder of the copied list. For all elements with the same 'id' (line 6) the amounts are summed into the 'reference' variable (line 7). Line 8 removes the element whose 'amount' was added to the other element from the original list (you were removing to wrong one here).

danpost

2015/2/23

#

There is something still missing in the code I provided above. The for loop should only execute if the 'id' of the current reference has not been dealt with yet. That is, we do not want a previously used 'checking' element to be used as a 'reference' element later. I believe that if we change the dealt with item elements to null in the 'list' list and just check for null, we can solve the issue.

public void fuseItems() {
    ArrayList<Item> list = new ArrayList<Item>(item);
    while (! list.isEmpty()) {
        Item reference = list.remove(0);
        if (reference != null) {
            for (Item checking : list) {
                if (checking != null && reference.id == checking.id) {
                    reference.amount += checking.amount;
                    item.remove(checking);
                    list.set(list.getIndex(checking), null);
                }
            }
        }
    }
}

Hopefully, that will work.

danpost

2015/2/23

#

I would guess you could do the same without creating a copy of the 'item' list using something like the following within the method:

for (int i=0; i<item.size()-1; i++) {
    Item reference = item.get(i);
    if (reference == null) continue;
    for (int j=i+1; j<item.size(); j++) {
        Item checking = item.get(j);
        if (checking == null) continue;
        if (checking.id == reference.id) {
            reference.amount += checking.amount;
            item.set(j, null);
        }
    }
}
while (item.contains(null)) {
    item.remove(null);
}

This was tested and confirmed okay.

fejfo

2015/2/23

#

huge thanks for all of this I will test it now

fejfo

2015/2/23

#

danpost wrote...

There is something still missing in the code I provided above. The for loop should only execute if the 'id' of the current reference has not been dealt with yet. That is, we do not want a previously used 'checking' element to be used as a 'reference' element later. I believe that if we change the dealt with item elements to null in the 'list' list and just check for null, we can solve the issue.

public void fuseItems() {
    ArrayList<Item> list = new ArrayList<Item>(item);
    while (! list.isEmpty()) {
        Item reference = list.remove(0);
        if (reference != null) {
            for (Item checking : list) {
                if (checking != null && reference.id == checking.id) {
                    reference.amount += checking.amount;
                    item.remove(checking);
                    list.set(list.getIndex(checking), null);
                }
            }
        }
    }
}

Hopefully, that will work.

list.set(list.getIndex(checking), null);

gives a compileerror but

checking = null;

compiles

danpost

2015/2/23

#

fejfo wrote...

list.set(list.getIndex(checking), null);

gives a compileerror

I misnamed the method. It is not 'getIndex', but 'indexOf'. Also, 'checking = null;' does not produce the same results as what was intended. That would only change the local variable 'checking' to null -- it would not change the element in the list to null. However, I would suggest using my tested code (from my latest code post).

fejfo

2015/2/23

#

it works

fejfo

2015/2/23

#

now I have a problem using the constructor of GreenfootImage to display the Items

import greenfoot.*;  // (World, Actor, GreenfootImage, Greenfoot and MouseInfo)
import java.io.*;
import java.awt.*;
/**
 * Write a description of class Item here.
 * 
 * @author (your name) 
 * @version (a version number or a date)
 */
public class Item extends Actor implements Serializable
{
    int amount;
    String id;
    boolean printedImageE = false;
    public Item(String newID,int newAmount) 
    {
        id = newID;
        amount = newAmount;
    } 

    public Item(String newID) 
    {
        id = newID;
        amount = 1;
    } 

    public String toString() {
        return "\nItem: id: "+id+"\t amount: "+amount;
    }    

    protected void addedToWorld(World Main) { // the porblem is in this method
        setImageToID();
        GreenfootImage idPart = getImage();
        idPart.scale(15,15);
        GreenfootImage textPart = GreenfootImage(""+amount,15,Color.yellow,null); //at this line
        GreenfootImage total = GreenfootImage(25,15);
        total.drawImage(idPart,0,0);
        total.drawimage(textPart,15,0);                
    }

    public void setImageToID() {

        IoHelp io = Main.io;
        try {
            setImage(id+".png");  
            printedImageE = false;
        }
        catch(java.lang.IllegalArgumentException e) {
            if(!printedImageE) {
                System.out.println("can't find deafault image of: " + id+".png");
                printedImageE = true;
            } 
        }

        try {
            setImage(io.image+id+".png");
        }
        catch(java.lang.IllegalArgumentException e) {}
    }
}

danpost

2015/2/23

#

Aren't you missing the keyword 'new' on that line !?! (and the next line) That is what tells the system to execute the constructor of that class.

fejfo

2015/2/23

#

ow I'm so stupid I knew u needed it

Activity

About

Documentation

Download

Discuss

Scenarios

java.util.ConcurrentModificationException what is this ? and how can I work around it