Enter a String and see if the String matches with the any of the Strings in an array (JAVA) [closed]

Question

I have created a class with the objective of having a method, within that class that receives a String x, and than it goest through a loop to see if the string received matches with any of the Strings inside the String[]. Here is my code:

public class MatchCountry
{
  public boolean findCountry(String a)
  {
    boolean match = false;
    String [] euCountries = {"Albania", "Andorra", "Armenia", "Austria", "Azerbaijan", "Belarus", "Belgium", "Bosnia and Herzegovina",
      "Bulgaria", "Croatia", "Cyprus", "Czech Republic", "Denmark", "Estonia", "Finland", "France", "Georgia", "Germany", "Greece",
      "Holland", "Iceland", "Ireland", "Italy", "Latvia", "Liechtenstein", "Lithuania", "Luxembourg", "Macedonia", "Malta",
      "Moldova", "Monaco", "Montenegro", "Netherlands", "Norway", "Poland", "Portugal", "Romania","Russia","San Marino",
      "Serbia", "Slovakia", "Slovenia", "Spain", "Sweden", "Switzerland", "Turkey", "Ukraine", "United Kingdom", "Vatican City"};
    int l = euCountries.length;

    for (int i = 0; i < l; i++)
    {
      System.out.println(euCountries[i]);
      if (a == euCountries[i])
        match = true;
      else
        match = false;
    }
    return match;
  }

  public static void main (String args[])
  {
    MatchCountry mc = new MatchCountry();
    boolean found = mc.findCountry("Portugal");
    System.out.println(found);
  }
}

Shouldn't this work? When I output the boolean found, it keeps giving me FALSE...

Answer 1

Instead of this:

  if (a == euCountries[i])
    match = true;
  else
    match = false;

put this:

  if (a.equals(euCountries[i])) {
    match = true;
  }

The problems with your if were:

• You were doing match = false when the string was not equal. This would reset any previously found matches, unless the country you are searching was the last in your list
• Strings should be compared with .equals method, otherwise you are comparing string references, not strings themselves.

Answer 2

Java strings are reference types, and comparing using == compares the values of those references. Unless the two Strings are the exact same object, the comparison will return false.

Try using euCountries[i].equals(a) rather than a == euCountries[i]. String.equals compares by value rather than by reference. (I've reversed the comparison because of one pitfall with using .equals: if a is null, then calling a.equals(anything) will throw a null pointer exception. The other way around, you know the string's not null, and equals will return false.)

You could make things a bit more efficient by saying like

for (int i = 0; i < l; i++) {
    if (euCountries[i].equals(a)) {
        return true;
    }
}
return false;

By doing that, you exit as soon as you've found a match, rather than looping through the whole array every time. You'll definitely want to get rid of the else { match = false; } whatever you do, though -- it will cause trouble for any but the last country in your array.

Also, you might consider using a HashSet rather than an array. It's much more efficient to check whether a string is in the set by hash code than it is to compare each element of an array. If you do that, though, you'll probably want it to be static and outside the function.

Answer 3

If your array of country codes is properly sorted, why not use java.lang.Arrays.binarySearch which saves you the trouble of having to loop yourself and is part of the standard JRE? I use it all the time in comparable cases.

Answer 4

Best would be to use a set implementation e.g. HashSet. However, even when there is a reason not to use sets, your code is far from optimal

First of all, put pure constant data into the class (static)

private static final String [] euCountries = {"Albania", "Andorra", "Armenia", "Austria", "Azerbaijan", "Belarus", "Belgium", "Bosnia and Herzegovina",
  "Bulgaria", "Croatia", "Cyprus", "Czech Republic", "Denmark", "Estonia", "Finland", "France", "Georgia", "Germany", "Greece",
  "Holland", "Iceland", "Ireland", "Italy", "Latvia", "Liechtenstein", "Lithuania", "Luxembourg", "Macedonia", "Malta",
  "Moldova", "Monaco", "Montenegro", "Netherlands", "Norway", "Poland", "Portugal", "Romania","Russia","San Marino",
  "Serbia", "Slovakia", "Slovenia", "Spain", "Sweden", "Switzerland", "Turkey", "Ukraine", "United Kingdom", "Vatican City"};

this version is much more efficient, no extra var, since data is ordered, you know when to stop

public boolean findCountry(String a)
{
  for (String country : euCountries) {
    int res = country.compareTo (a);  // or use compareToIgnoreCase when more appropriate
    if (res == 0) return true;
    if (res > 0) return false;
  }
  return false;
}

Next optimization step would be to build a binary search

Answer 5

Try this:

for (int i = 0; i < l; i++)
{
    if (a.equals(euCountries[i])) {
        return true;
    }
}
return false;

It's more efficient to exit as soon as you find a match. Why keep looking?

Here's another thing to try that might make it simpler: instead of storing the countries in an array, put them in a List and check it this way:

return (euCountries.contains(a));