Return a value present in loop in java

Question

I want to return a value from the loop. My code is as follows:

public long findLocationId(String location) throws SystemException
    {

        long locId = 1;

        List<Location> locationList = LocationLocalServiceUtil.getLocations(-1,-1);

            for(Location findLoc : locationList)  {
                if(location == findLoc.getLocationName())  {
                    locId = findLoc.getLocationId();

                    **return locId;**
                }
            }

    }

When I try to put the return value in loop I get an error saying to include a return statement.

How should I change my code so that I can return a value from the loop itself?

I want to return the new locId value which I get in the loop and not the value which I set initially as locId = 1; I want to return the new value of locId which i get from the loop

Answer 1

There are various approach to this problem.

1. Use while loop
2. Use for loop with additional stop condition
3. Use break key word.

Let first create a template before introduce our logic:

public long findLocationId(String locationName) throws SystemException
    {

        if(locationName == null) { //Here we cover first issue. 
            throw new IllegalArgumentException("THe locationName must not be null");
        }

        long locId = Long.MIN_VALUE; //We declare a default value that will be returned if none match found. 

        Collection<Location> locationList = getLocationList(); //The location can be read from another method so we are not binded to field.

        if(locationList == null || locationList.isEmpty()) {
            return locId; // Or throw an exception about invalid state.
        }


        //Place for the logic


        return locId;

    }

---

Typically when we do not know when we want to stop the iteration, it is a sign that we should start with while loop.

So lets try it.

Solution 1 - The while way.

 Iterator<Location> iterator = locationList.iterator();

        while(iterator.hasNext() && Long.MIN_VALUE != locId) {

            Location location = iterator.next();

            if(locationName.equalsIgnoreCase(location.getLocationName())) {
                locId = location.getLocationId(); // This will change the locId, so second condition will be no longer true and loop will end.
            }

        }

The pros: - It works

The cons: - Leave the iterator

We should not leave iterators, as this is error prone. This lead us to next solution.

Solution 2 - We use the pattern for iterator instead of while.

 for(Iterator<Location> iterator2 = locationList.iterator();iterator.hasNext() && Long.MIN_VALUE != locId;) {

            Location location = iterator.next();

            if(locationName.equalsIgnoreCase(location.getLocationName())) {
                locId = location.getLocationId(); // This will change the locId, so second condition will be no longer true and loop will end.
            }
        }

Pros - It works

Cons - It complicated, we must thing about the stop, when reading this code.

As above solution is not easy to read is should be also removed.

Solution 3 - Why break is useful.

for(Location location : locationList) {

            if(locationName.equalsIgnoreCase(location.getLocationName())) {
                locId = location.getLocationId();
                break;
            }

        }

Pros - It works - It readable

Cons - None

Conclusion is that the code should be readable. Using break, we point that we found the match and we do not want to progress anymore.

Fine. But what about the case when location was found ?

OP example we return 1L. This is not the best choice as is very likely that this value could be used as ID.

In previous examples i have used the min value of long. This is acceptable for some cases but, still we need to validate the method result, and also document it.

The final solution present additional loop exit, that is return key word.

public long findLocationId(String locationName) throws SystemException
    {

        if(locationName == null) { //Here we cover fist issue. 
            throw new IllegalArgumentException("THe locationName must not be null");
        }

        Collection<Location> locationList = getLocationList(); //The location can be read from another method so we are not binded to field.

        if(locationList == null) {
            throw new IllegalStateException("THe location list was not initialized"); 
        }

        for(Location location : locationList) {

            if(locationName.equalsIgnoreCase(location.getLocationName())) {
                return location.getLocationId(); //We exit from the method.
            }

        }

        throw new SystemException("Could not found location for name:" + locationName); 

    }

Additional note

In the example OP have location == findLoc.getLocationName(), the problem with this code is that we should not use == to compare objects types (details). As we deal with String class recommended method is to use method String#equals(Object) or 'String#equalsIgnoreCase(String)'. For this example i have used the second option.

Answer 2

It is because there isn't always going to be a case when location == findLoc.getLocationName(). Even if that IS the case, the java compiler doesn't know what sort of input you are giving your program so it it telling you that there could be a case when the function doesn't return anything even though it MUST return a long.

Just return -1L or something that your pogram can consider to be "not found" at the end of the function.