up vote 0 down vote favorite

This will be difficult to explain but I shall try my best.

I have 3 classes and a main class. The classes are City, Tour and Population. A population has an arraylist of X amount of tours and tour has an array list of X amount of cities. Where a City is just an X and Y coordinate. When I create new tours the cities within them are randomises and then i add the tour to the population arraylist and this is working fine, the outputs are randomised. However if I then make output the arraylist again they are no longer randomised and all the tours are the same. I think this will be easier to explain by showing some code:

Population.java

public class Population {

private ArrayList<Tour> toursList;

public Population(int size, ArrayList<City> loadedCities)
{
    toursList = new ArrayList<Tour>();

    // Fill the population with the number of tours
    for (int i = 0; i < size; i++) {
        Tour newTour = new Tour(loadedCities);
        newTour.setId(i);
        toursList.add(newTour);
    }

}

public void output() 
{
    for (Tour tour : toursList)
        System.out.println(tour.toString());
}
}

Tour.java

public class Tour {

private ArrayList<City> tour;
private int id = 0;

public Tour(ArrayList<City> cities)
{
    Collections.shuffle(cities);
    tour = cities;
}

public void setId(int i)
{
    this.id = i;
    System.out.println("Constructor: "+toString());
}

public int getId()
{
    return id;
}

public String toString()
{
    String str = "Tour: "+id+" - ";
    for (City city : tour) {
         str += city.toString()+" | ";
    }
    return str;
}
}

City.java

public class City {

private int code;
private Double y;
private Double x;

public City(int code, Double y, Double x)
{
    this.code = code;
    this.y = y;
    this.x = x;
}

public int getCode() 
{
    return code;
}

public Double getX() 
{
    return x;
}

public Double getY()
{
    return y;
}

public String toString()
{
    return "Code: "+this.code+" - X: "+this.x+" Y: "+this.y;
}
}

And then the main class just makes these calls after loading the cities arraylist:

Population population = new Population(10, cities);
population.output();

The console output for the few println's are as follows (stripped down version):

Constructor: Tour: 0 - Code: 2 - X: 42373.8889 Y: 11108.6111
Constructor: Tour: 1 - Code: 28 - X: 43026.1111 Y: 11973.0556
Tour: 0 - Code: 8 - X: 42983.3333 Y: 11416.6667
Tour: 1 - Code: 8 - X: 42983.3333 Y: 11416.6667

You can see the tours have now all become the same and in the same order.

Please let me know if you need any more information or need me to explain the issue clearer.

Many Thanks.

share|improve this question
90% accept rate
Size is used to configure how many tours there are, this will be an option on the interface to increase the number of tours. – Boreded Oct 23 at 17:53
Ah, I see; I wasn't paying attention. – Dave Newton Oct 23 at 18:03
feedback

2 Answers

up vote 2 down vote accepted

You use the same ArrayList<City> cities for all tours.

Tour constructor should be like this:

tour = new ArrayList<City>(cities);
Collections.shuffle(tour);
share|improve this answer
Of course. I knew it would be something simple but I just could not see it! Thank you! p.s. Will accept your answer in 3 minutes when the system lets me :) – Boreded Oct 23 at 17:52
feedback
up vote 0 down vote

Java uses mutable collections. Since you're passing in the same collection into all the Tours it gets shuffled a lot of times, but in the end all tours have a reference to the same, much shuffled, collection.

Two things to learn from this:

  1. if you want to modify a collection you get passed in, first make a copy (e.g. new ArrayList(collectionToCopy))
  2. if you pass a collection you own to someone else, first make sure he can't change it (e.g. Collections.unmodifiableList(myCollection))

You'll step into this trap a lot of times in your Java career. And if you don't, someone else will do it for you.

share|improve this answer
feedback

Your Answer

 
or
required, but never shown
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.