Searching an array list for a specific record in java

Question

Im writing a method to return a specific record in an array however it throws up two errors and Im not sure how to fix it. Can anyone explain what I'm doing wrong?

public String find(String searchName) 
{ // ERROR - MISSING RETURN STATEMENT
    Iterator<TelEntry> iterator = Directory.entries.iterator();
    boolean hasFound = false;
    while (iterator.hasNext()) 
    {
        TelEntry entry = iterator.next();

        if (entry.name.equalsIgnoreCase(searchName)) {
            return entry.name + entry.telNo;
            hasFound = true; // ERROR UNREACHABLE STATEMENT
        }

    }
    if (hasFound==false)
    {
        System.out.println("sorry, there is noone by that name in the Directory. Check your spelling and try again");
    }
}

Can anyone explain what I've done wrong?

Answer 1

The basic problem you have is that when a match is not found, you have no return statement. Usually, a method will return null is such cases, but you may want to return searchName, or even the error message - it depends on what the intention/contract of the method is (not stated).

However, the other problem you have is that your code is way too complicated for what it's doing, especially the hasFound variable is completely useless.

Change your code to this, which does exactly the same thing but is expressed more elegantly:

public String find(String searchName) {
    for (TelEntry entry : Directory.entries) {
        if (entry.name.equalsIgnoreCase(searchName)) {
            return entry.name + entry.telNo;
        }
    }
    System.out.println("sorry, there is noone by that name in the Directory. Check your spelling and try again");
    return null; // or return "searchName", the error message, or something else
}

Answer 2

Your code is incorrect : you cannot have instructions after a return in the same block: how could it be executed, since the function has returned ... ?

---

That's what the compiler is telling you : unreachable statement

Answer 3

return statement should be last staetement in a block. Change your code below:

if (entry.name.equalsIgnoreCase(searchName)) {
            hasFound = true; // ERROR UNREACHABLE STATEMENT
            return entry.name + entry.telNo;
        }

Answer 4

Prefer return the string itself. Indeed, print it to screen directly violates SRP (Single Responsibilty principle and also avoid to return something as the method expects).

No need for boolean checker.

public String find(String searchName) { 
        Iterator<TelEntry> iterator = Directory.entries.iterator();
        while (iterator.hasNext()) {
            TelEntry entry = iterator.next();
            if (entry.name.equalsIgnoreCase(searchName)) {
                return entry.name + entry.telNo;
            }
        }
        return "sorry, there is noone by that name in the Directory. Check your spelling and try again";
}

Answer 5

If a method is declared as returning a String, it must return a String, or throw an exception. Not returning anything is not acceptable. So you should decide what to do when the string is not found. You basically have two choices:

• returning null
• throwing an exception

Printing an error is not a good idea. Such a method shouldn't deal with the user interface. That's not its responsibility, and a method should only have one responsibility. Returning a string with an error message is not a good idea either: the caller would have no way to know if the returned string is the one which has been found, or an error string.

Moreover your code is overly complicated. It could be reduced to the following (assuming Directory.entries() implements Iterable, as it should):

public String find(String searchName) {
    for (TelEntry entry: Directory.entries()) {
        if (entry.name.equalsIgnoreCase(searchName)) {
             return entry.name + entry.telNo;
        }
    }
    return null;
}

I would change the return type, so, and make it return a TelEntry instance. Let the caller deal with the concatenation. It's not the responsibility of this method either.

Answer 6

Can anyone explain what I've done wrong?

My explanations are in comments below:

public String find(String searchName) 
    { // ERROR - MISSING RETURN STATEMENT
        Iterator<TelEntry> iterator = Directory.entries.iterator();
        boolean hasFound = false;
        while (iterator.hasNext()) 
        {
            TelEntry entry = iterator.next();

            if (entry.name.equalsIgnoreCase(searchName)) {
                return entry.name + entry.telNo;
                /*The "return" statement above stops executing
                the current method and transfers control to the 
                place from where "find" method was called. The
                statement below this line is NEVER executed.*/
                hasFound = true; // ERROR UNREACHABLE STATEMENT
            }

        }
        if (hasFound==false)
        {
            System.out.println("sorry, there is noone by that name in the Directory. Check your spelling and try again");
        }
        /*This method starts with "public String". That means that 
        it MUST return a String object or null when the method 
        finishes executing.
        The best place to return is here.*/
    }