fastest way to find a nested element

Question

I have two classes:

1. country
2. country_name

Country.java

public class Country {
 private Integer ID;
 private String ISO;
 public Integer getID() {
 return ID;
 }
 public void setID(Integer ID) {
 this.ID = ID;
 }
 public String getISO() {
 return ISO;
 }
 public void setISO(String ISO) {
 this.ISO = ISO;
 }
}

Country_Name.java

public class Country_Name {
 private Integer ID;
 private Integer COUNTRY_ID;
 private String NAME;
 public Integer getID() {
 return ID;
 }
 public void setID(Integer ID) {
 this.ID = ID;
 }
 public Integer getCOUNTRY_ID() {
 return COUNTRY_ID;
 }
 public void setCOUNTRY_ID(Integer COUNTRY_ID) {
 this.COUNTRY_ID = COUNTRY_ID;
 }
 public String getNAME() {
 return NAME;
 }
 public void setNAME(String NAME) {
 this.NAME = NAME;
 }
}

Each country has an ID and ISO code. Each country_name can have multiple names relating to any given country.ID.

I add the data in a similar fashion to this (Please note that this is not the focus of the question though):

public static void main(String args[]) {
 Country country = new Country();
 country.setID(1);
 country.setISO("USA");
 Country_Name country_name1 = new Country_Name();
 country_name1.setID(1);
 country_name1.setCOUNTRY_ID(1);
 country_name1.setNAME("United States");
 Country_Name country_name2 = new Country_Name();
 country_name2.setID(2);
 country_name2.setCOUNTRY_ID(1);
 country_name2.setNAME("US");
 }

I'm trying to do something similar to a JOIN I guess but without using a database.

This is what I want reviewed:

public static final synchronized Country get(final List<Country_Name> allCountry_Names, final List<Country> allCountries, final String NAME) {
 for (Country_Name country_name : allCountry_Names) {
 if (country_name.getNAME().equals(NAME)) {
 for (Country country : allCountries) {
 if (country_name.getCountryID().equals(country.getID())) {
 return country;
 }
 }
 }
 }
 return null;
 }

Could this be improved in any way?

• Pimgd: Your classes are wrong and need a redesign (why the hell is a country_name a class? Did you start database first? ElementCollection is a thing, look it up in JPA spec). So does this method. This code requires more context to get a proper answer.
• Hooli: Not looking to get into JPA or anything like that. Can you be more specific on why the design is wrong? A single country may have multiple names, what's wrong with that?
• radarbob: Build a HashTable of List<Country_Name> with the ID as the key of course. I assume the List<Country> are all unique so a HashTable here is not helpful. The more country_names per ID, the more efficient the HashTable is over the given code. HOWEVER, if the data is coming from a DB, why not join it there and have a Country class containing a List<Country_Name>?

Answer 1

I think you should re-design your code. You need first to follow language guideline about the class names and attribute names, following the camel case.

The list of countries names could be an attribute of the class Country, you don't need a specific class to handle just this info.

So the class Country_Name will be removed.

Country

public class Country {
 private Integer id;
 private String iso;
 private List<String> names;
 public Integer getId() {
 return ID;
 }
 public void setId(Integer id) {
 this.id = id;
 }
 public String getIso() {
 return iso;
 }
 public void setIso(String iso) {
 this.iso = iso;
 }
 public List<String> getNames() {
 return names;
 }
 public void setNames(List<String> names) {
 this.names = names;
 }
 public boolean hasName(String countryName) {
 return this.names.contains(countryName);
 }
}

So if you want to maintain the things as simple as possible, you can change your get() method like so:

public static final synchronized Country get(final List<Country> allCountries, final String name) {
 for (Country country : allCountries) {
 if (country.hasName(name)) {
 return country;
 }
 }
 return null;
}

This is better (fasters) than your code, as it uses just one loop, instead of 2 nested.

With this solution you don't need to add code to initialize data structures and don't have to allocate further memory to handle it.

The disadvantage is in the fact that you have to loop on the collection each time you look for a country.

Or to be more fast, you could build an HashMap like so: Map< String, Country> namesForCountries that you put in a configuration object or a singleton you share on your classes.

You have to initialize it so:

public static void init(List<Country> allCountries) {
 ...
 Map<String, Country> namesForCountries = new HashMap<>();
 for (Country country : allCountries) {
 for (String name : country.getNames()) {
 namesForCountries.put(name, country);
 }
 }
 ...
}

And your get() method will became:

public static Country get(String name) {
 if (namesForCountries.contains(name)) {
 return namesForCountries.get(name);
 }
 return null;
}

Or simply you don't need it at all.

The HashMap is faster, but pay in the initialization time, and you could use in case where the data should not change in runtime.

The HashMap performace are connected to the initial capacity and the load factor.

The initial capacity is just the number of bucket in the hash table. The load factor is a number used from the vm to when should allocate new bucket for the hash table. In short it is a measure of how full the hash table is.

In your case, I assume you could load all countries on start-up, and then you never need to change it.

If so, the HashMap is faster.

If you need to change the HashMap runtime, than consider to use the synchronized one with:

Map m = Collections.synchronizedMap(new HashMap(...));

As the HashMap is not synchronized.

• Hooli: What is your reasoning for choosing HashMap get() over List iteration?
• Mario Santini: I updated my answer, please have a look.
• Simon Forsberg: To improve the speed even further, I would recommend using private Set<String> names; for the Country. Ask yourself the questions: 1. Should duplicates be allowed? 2. Does order matter? If both the answers is "No" then what you should use is a Set, not a List
• Hooli: @MarioAlexandroSantini: I take that it's init time is slower because it creates indexes which improves runtime performance. Is that accurate?
• Mario Santini: @Hooli in your case don't change so much. Generally speaking it costs as you have to initialize an HashMap object that allocate memory, I don't mean the copy of the elements, objects are referenced, but the HashMap itselfs need memory and depends on the number of elements. Than have to indexing the element while you add. But in change, after that you have a very fast access to the objects.