return statement inside try catch and if else clause

Question

The problem is that something strange is going on inside my code; Let's go to my example (i cleaned it a bit):

public int foo() throws IOException {
        if(redirect_url.indexOf("statement_1") != -1){
            if(check() == true){
                //do something
            }else{
               //do something
                foo(); //yep, recursive call
            }
        }else if(redirect_url.indexOf("statement_2") != -1) {
            map.clear();
            bar();
            map.put("val", 1);
            map.put("val2", "2");
            foo(); //yep, recursive call; Remember this line!!
        }else if(redirect_url.indexOf("statement3") != -1) {
            return AUTH_SUCCESS;
        }else if(redirect_url.indexOf("statement4") != -1){
            return AUTH_SUCCESS;
        }else {
            return AUTH_FAILED;
        }

    }catch (Exception e){
        return AUTH_FAILED;
    }
    return AUTH_FAILED;
}

There is a little function that is called by another function, let's call it buzz

public void buzz(){
     try {
         switch (signInAttempt()){
             case AUTH_SUCCESS:
                 //do smth
                 return true;
             case AUTH_FAILED:
                 //do smth
                 return false;
             case ACCESS_REQUEST:
                    //do smth
             default:
                 return false;
             }
      } catch (IOException e) {
            e.printStackTrace();
        }
}

I've unpacked my awesome debugger when noticed that there were logical errors in my code and found out funny thing.

Let's say that redirect_url string has "statement4" substring, so that the fourth elseif clause (don't count inner elseif clause) will go inside and return AUTH_SUCCESS. I thought so.

And here is the issues, when the return AUTH_FAILED is triggered, the next instruction is calling foo() function inside second else if statement. I do not have any idea why this could happened. So strange. And ideas?

UPD 1: Constants defined inside the class: Example

private static final int AUTH_SUCCESS      = 4;

UPD 2 Some more code:

Meet the calling function

public boolean rollIn(){

        try {
            switch (signInAttempt()){
                case AUTH_SUCCESS:
                    //do smth
                case AUTH_FAILED:
                    return false;
                case ACCESS_REQUEST:
                    return true;
                default:
                    return false;
            }
        } catch (IOException e) {
            e.printStackTrace();
        }

        return true;
    }

And finally meet the patient:

 public int signInAttempt() throws IOException {

        try {

            /*connection*/
            this.doc = connection.parse();

            System.out.println(redirect_url);
            if(redirect_url.indexOf("authorize") != -1){
                if(findCaptcha() == true){
                    signInAttempt();
                }else{
                    authData.clear();
                    signInAttempt();
                }
            }else if(redirect_url.indexOf("authcheck") != -1) {
                authData.clear();
                authData.put("v1", 1);
                authData.put("v2", 2);
                System.out.println(action_url);
                signInAttempt();
            }else if(redirect_url.indexOf("__q_hash") != -1) {
                System.out.println("AUTH SUCCESS");
                return AUTH_SUCCESS;
            }else if(redirect_url.indexOf("access_token") != -1){
                return AUTH_SUCCESS;
            }else {
                return AUTH_FAILED;
            }

        }catch (Exception e){
            return AUTH_FAILED;
        }
        return AUTH_FAILED;
    }

Something like this

Your question is unclear at the moment. It would be easier to help you if you'd provide a [mcve]. — Jon Skeet, Jul 21 '16 at 21:42
Your methods don't take any input, but your if-statements rely on the current state. It's tough to debug this type of code. Consider re-factoring your code such that the method only utilizes passed in arguments. — FishStix, Jul 21 '16 at 21:44
@FishStix , well, I have had started of course with functions that have inputs, but later I found out that this functions often use common values and I put that into class fields. Yep, I see how the code is shitty :( — Ascelhem, Jul 21 '16 at 21:53
@JaroslawPawlak, well, i did it and it fixed my problem. You should post this as an answer, also I want a little explanation. — Ascelhem, Jul 21 '16 at 22:00
ALSO any recommendation about code/algorithm would be highly appreciated! The purpose of the code is to authorize. 1. Take form data, 2. try to auth; 3. Check if captcha exits (auth again) with it; 4. follow redirect and some other options; — Ascelhem, Jul 21 '16 at 22:18
@xenteros It's a typo - it's not actual code, but a simplified example. — Jaroslaw Pawlak, Jul 22 '16 at 15:43

score 0 · Answer 1 · answered Jul 22 '16 at 13:31

In places where you do recursive calls, you should have return foo();.

A short explanation how it works now - without return:

the method gets called for the first time
it makes a recursive call
recursive call executes and returns a value so we are back in the scope of the first call
the value returned by recursive call is ignored - you don't assign it or return it
The execution continues to outside the if-statement, no exception caught so it goes to your last statement return AUTH_FAILED;

So even, if recursive call returned AUTH_SUCCESS, the first call will ignore it and return AUTH_FAILED anyway.

return statement inside try catch and if else clause

1 Answers1