I always get the last value of an arraylist of objects

Question

I want to print all the items of an arrayList of objects. But it only prints the last item's values only. Here is my code:

List<items> boxes = new ArrayList<items>();
boxes.add(new items("1", 0.1f, 0.2f, 0.1f));
boxes.add(new items("2", 0.1f, 0.4f, 0.1f));
boxes.add(new items("3", 0.1f, 0.1f, 0.2f));
boxes.add(new items("4", 0.1f,0.1f, 0.3f));
boxes.add(new items("5", 0.2f, 0.1f, 0.1f));
boxes.add(new items("6", 0.1f, 0.1f, 0.1f));
boxes.add(new items("7", 0.2f, 0.3f, 0.1f));
boxes.add(new items("8", 0.1f, 0.3f, 0.1f));
boxes.add(new items("9", 0.2f, 0.2f, 0.2f));

for (items box: boxes){
    System.out.println();
    System.out.println("length:  " +box.dimension.get("length"));
    System.out.println("breadth:  " +box.dimension.get("breadth"));
    System.out.println("height:  " +box.dimension.get("height"));
}

this is the items class:

import java.util.*;

public class items {

    String boxNumber;
    public static Map<String, Float> dimension = new HashMap<String, Float>();
    public double volume;

    public items(float l, float b, float h) {
        volume = l * b * h;
        dimension.put("length", l);
        dimension.put("breadth", b);
        dimension.put("height", h);

    }

    public items(String boxName, float i, float j, float k) {
        boxNumber = boxName;
        volume = i * j * k;
        dimension.put("length", i);
        dimension.put("breadth", j);
        dimension.put("height", k);

    }

    public static Map<String, Float> getDimension() {
        return dimension;
    }

    public static void setDimension(Map<String, Float> dimension) {
        items.dimension = dimension;
    }

    public items rotateBox() {

        Set<String> keySet = this.dimension.keySet();
        String[] sides = keySet.toArray(new String[3]);
        dimension.put(sides[1],
                dimension.put(sides[2], dimension.get(sides[1])));
        dimension.put(sides[0],
                dimension.put(sides[2], dimension.get(sides[0])));

        return this;
    }

    public double getVolume() {
        // TODO Auto-generated method stub
        return dimension.get("length") * dimension.get("breadth")
                * dimension.get("height");
    }
}

And this is what I always get:

length:  0.2
breadth:  0.2
height:  0.2

length:  0.2
breadth:  0.2
height:  0.2

length:  0.2
breadth:  0.2
height:  0.2

length:  0.2
breadth:  0.2
height:  0.2

length:  0.2
breadth:  0.2
height:  0.2

length:  0.2
breadth:  0.2
height:  0.2

length:  0.2
breadth:  0.2
height:  0.2

length:  0.2
breadth:  0.2
height:  0.2

length:  0.2
breadth:  0.2
height:  0.2

Can anyone please tell what is wrong with my code?

Don't use `static` unless you want all instances to share the same values. — Peter Lawrey, Feb 26 '16 at 10:08
Don't use a map when you should be using a normal object with fields. This performs badly, and is error prone. — Peter Lawrey, Feb 26 '16 at 10:09

score 3 · Accepted Answer · edited May 23 '17 at 11:50

You have declared dimension as a static variable, which means it will be shared by all instances of your items class.

public static Map<String, Float> dimension = new HashMap<String, Float>();

As such, every time you create a new instance, the values in the map are overwritten by

dimension.put("length", l);
dimension.put("breadth", b);
dimension.put("height", h);

so the final values you will get are the values of the last item you inserted.

You should instead make it an instance variable with:

public Map<String, Float> dimension = new HashMap<String, Float>();

As a side-note:

You should prefer private instance variable over public ones, and have getter.
Consider respecting the Java naming conventions: the items class should be Items.

score 0 · Answer 2 · answered Feb 26 '16 at 10:10

You have made HashMap dimension as a static variable so it will retain its value and is shared by all the instances of items class. Make the Map as the member variable by declaring it as

    public Map<String,Float> dimension = new HashMap<String,Float>();

This member variable will be specific to every instance of the Class items and can be accessed from constructor using "this"keyword. For example:-

    public items(String boxName, float i, float j, float k) {
        boxNumber = boxName;
        volume = i * j * k;
        this.dimension.put("length", i);
        this.dimension.put("breadth", j);
        this.dimension.put("height", k);
    }

I always get the last value of an arraylist of objects

2 Answers2