Java ArrayList in Junit test filled with single value

Question

i have the following Junit class for testing (tests are not included sinc they are not important in this case):

public class BoardImplTest {

List<Coords> startingCells = new ArrayList<>();
Coords cellCoords = new Coords();


private void addCell(int col, int row){
    cellCoords.setCols(col);
    cellCoords.setRows(row);

    startingCells.add(cellCoords);
}

private void testSetup() {
    addCell(3,2);       

    addCell(4,2);

    addCell(1,3);

    addCell(2,3);

    addCell(3,3);

    addCell(4,4);   

}

after running is, the array startingCells is filled with only one value:

after first addCell(3,2); array has size=1 with one element of coords of (3,2)
after addCell(4,2); array has size=2 with two elements of coords of (4,2)
after addCell(1,3); array has size=3 with three elements of coords of (1,3)

and so on.

after every call of addCell, array increases its size and all fields are filled with values passed as argument, and i want these values only to be added as the last element of Array. Where is the problem?

Side note on code quality: depending on your use case, you actually really want to follow Jon's advise and make sure that a Coord is always created with x and y. You see: just creating an empty object, and adding "necessary" property later on is simply dangerous. And if the idea of "Coord" is to represent a two dimensional point, then only give it a constructor that takes an x and y position for example. And think if "rows" and "cols" are really good names for the "aspects" that those object properties represent. — GhostCat, Jul 25 '16 at 09:29

Jon Skeet · Accepted Answer · 2016-07-25T09:36:50.213

6

You're creating a single instance of Coords, and adding a reference to that single object every time addCell is called, after mutating it. You should create a new instance of Coords each time, e.g.

// Remove the cellCoords field

private void addCell(int col, int row) {
    Coords cellCoords = new Coords(col, row);    
    startingCells.add(cellCoords);
}

(That's assuming a sensible Coords constructor. If there isn't one, you'll need to create an instance and then call setCols and setRows... but still you'll be creating a new object each time. If Coords is under your control, you can make sure that it does have a suitable constructor, and make it more readable as Coordinate, with column and row properties, or even x and y... I'd advise making it final and immutable too.)

edited Jul 25 '16 at 09:36

answered Jul 25 '16 at 09:23

Jon Skeet

1,421,763
867
9,128
9,194

thank you, forgot its only passing reference. it solved the problem. – Akka Jaworek Jul 25 '16 at 09:25
1

Just wondering: how does this work ... I mean: your answer is OK, but well, not extraordinary. You see, such things have been asked often before, and typically, a similar answer gets maybe 2,3 upvotes, like http://stackoverflow.com/questions/16515362/creating-new-instance-of-object-in-loop-to-add-in-list. You collect 6 upvotes within 10 minutes. So just wondering ... how do you do that? Is there some built-in magnetism in your postings that attract votes? No complaining here, just curiosity ... – GhostCat Jul 25 '16 at 09:32
1

@GhostCat: No idea. I don't think many people are tracking my every move, but possibly if there's something that's "bordeline useful" more people may upvote my content than that of others, which is certainly a shame. – Jon Skeet Jul 25 '16 at 09:37

Java ArrayList in Junit test filled with single value

1 Answers1