CODEWITHSUNDEEP
javafor-loop
aborted
aborted

Reputation: 4541

Correct way to return from inside of for loop in Java?

So I have this little method which takes an integer argument and checks if an array (which is set as an attribute) contains a certain object. If it does, it returns that object, otherwise it returns null. The problem here is that I get an error when I try to return something. Here it is:

myClass.java:33: error: missing return statement
        }
        ^
1 error

I do have a return statement and it includes the ternary operator, meaning that it returns something at any cost. Here's my method:

public Banesa gjejBanese(int nrBaneses) {

    for (int i = 0; i < listaBanesave.length; i++) {
        return (listaBanesave[i].getNrBanesa() == nrBaneses) ? listaBanesave[i] : null;
    }
}

What is wrong with it? Why do I get that error?

Upvotes: 0

Views: 552

Answers (7)

kai
kai

Reputation: 6887

Your logic is wrong too. You only check if the first item in the array is == the int you passed to the function. After checking the first item there is definitely a return doesn't matter which case is true (if or else) and the function ends. It is the same like:

public Banesa gjejBanese(int nrBaneses) {

    return (listaBanesave[0].getNrBanesa() == nrBaneses) ? listaBanesave[i] : null;

}

it should be:

public Banesa gjejBanese(int nrBaneses) {

    for (int i = 0; i < listaBanesave.length; i++) 
    {
        if(istaBanesave[i].getNrBanesa() == nrBaneses)     
            return listaBanesave[i];
    }
    return null;
}

Upvotes: 1

fge
fge

Reputation: 121830

takes an integer argument and checks if an array (which is set as an attribute) contains a certain object. If it does, it returns that object, otherwise it returns null.

Given your requirements, your code should be (for example):

public Banesa gjejBanese(int nrBaneses) {
    Banesa ret;
    for (int i = 0; i < listaBanesave.length; i++) {
        ret = listaBansesave[i];
        if (ret.getNrBanesa() == nrBaneses)
            return ret;
    }

    // No element found or list is empty
    return null;
}

With your current code:

  • if the list is empty, the "for" loop is not entered and therefore there is no return statement -- hence your compile error;
  • in any case you only ever check for the first element, you never check the others, since you return in each iteration (of which there will only be one)

Upvotes: 3

wmorrison365
wmorrison365

Reputation: 6318

In line with my comment, I'd prefer this to be written:

public Banesa gjejBanese(int nrBaneses) {
    Banesa found = null;
    for (int i = 0; i < listaBanesave.length; i++) {
        if (listaBanesave[i].getNrBanesa() == nrBaneses) {   // should this be == or equals()?
            found = listaBanesave[i];
            break;
        }
    }
    return found;
}

I just think it's clearer, there's one point of exit (rather than in a for loop). I'd say the ?: operator is very useful but that it doesn't assist readability in your scenario.

Also, I've left a comment to check if "==" is a sufficient test or should `#equals be user?

Upvotes: 0

Boris the Spider
Boris the Spider

Reputation: 61198

I think you need to loop over all items in listaBanesave and check if each meets the criteria and, if none of them do, return null. Currently your code returns null if the first item does not meet the criteria - i.e. it could be rewritten as:

public Banesa gjejBanese(int nrBaneses) {
    if(listaBanesave.length == 0)
        return null;
    return (listaBanesave[0].getNrBanesa() == nrBaneses) ? listaBanesave[0] : null;
}

Presumably this isn't what you want.

public Banesa gjejBanese(int nrBaneses) {
    for (int i = 0; i < listaBanesave.length; i++) {
        if (listaBanesave[i].getNrBanesa() == nrBaneses) {
            return listaBanesave[i];
        }
    }
    return null;
}

So check each item in the loop; it if matches, return it. Otherwise, once the loop is finished, you know you have no match - the return null.

Upvotes: 0

Suresh Atta
Suresh Atta

Reputation: 122026

There is a chance that the condition in loop doesn't satisfy and no returning might happen.

So you need to have a default return.

public Banesa gjejBanese(int nrBaneses) {

    for (int i = 0; i < listaBanesave.length; i++) {
        return (listaBanesave[i].getNrBanesa() == nrBaneses) ? listaBanesave[i] : null;
    }
   return null; //when there is no chance to enter in loop.
}

Upvotes: 0

Kaerber
Kaerber

Reputation: 1653

You should return null as the last line of the method, even according to your specification ("If it does, it returns that object, otherwise it returns null.").

public Banesa gjejBanese(int nrBaneses) {

    for (int i = 0; i < listaBanesave.length; i++) {
        return (listaBanesave[i].getNrBanesa() == nrBaneses) ? listaBanesave[i] : null;
    }

    return null;
}

Upvotes: 0

Maroun
Maroun

Reputation: 96018

You should have a return outside the loop. Because when listaBanesave is empty, there won't be a return statement - That what makes the compiler angry.

What to return is your decision.. Depends on your logic.

Upvotes: 1

Related Questions