A little about the correctness of writing code. Which option to choose?

Question

Java

Let's say we have a click handler like this:

@Override
    public void onButtonClick() {
        if (isRequested()) {
            if (rulesField.isChecked()) {
                register(numberField.getText().toString(), codeField.getText().toString());
            } else {
                showMessage(R.string.error_rules);
            }
        } else {
            requestRegister(numberField.getText().toString());
        }
    }

It seems to be nothing bad, but acquaintances do not particularly perceive this. What is the problem here?
They offered this option:

@Override
    public void onButtonClick() {
        if (isRequested() && rulesField.isChecked()) {
            register(numberField.getText().toString(), codeField.getText().toString());
            return;
        }
        if (isRequested() && !rulesField.isChecked()) {
            showMessage(R.string.error_rules);
            return;
        }
        if (!isRequested()) {
            requestRegister(numberField.getText().toString());
        }
    }

What do you think is the best way to write? Or does it not matter at all?
PS #1: I gave an example of a small construction, there may be more conditions.
PS #2: The question may be stupid, but for some reason it worries me :)

Answer 1

Many levels of nesting are hard to read. Try adding conditions inside

if (rulesField.isChecked()) {
    register(numberField.getText().toString(), codeField.getText().toString());
}

A couple more nested if-else to get this:

@Override
    public void onButtonClick() {
        if (isRequested()) {
            if (rulesField.isChecked()) {
                    if (canProceed()) {
                         register(numberField.getText().toString(), codeField.getText().toString());
                    } else if (isExtraRequestRequired()) {
                        doSomething();
                     } else if (oneMoreCondition()) {
                         doSmomethingOnExtraCondition();
                     } else {
                          Log.e("Unexpected condition");
                     }
            } else {
                showMessage(R.string.error_rules);
            }
        } else {
            requestRegister(numberField.getText().toString());
        }
    }

and you will understand that keeping in mind the branch that you are interested in (first if, then the else nested in it, then the second if-else from the first if nested in else) becomes absolutely impossible. Therefore, they try to structure the code in such a way that it is "flat", that is, without nesting. This, of course, is an ideal, and sometimes difficult to achieve, but it’s worth striving for - the person who will read your listing after you (and most often it will be you yourself two weeks after you wrote it, and successfully forgot the whole structure), will thank you.

Answer 2

what they suggested is worse
isRequested() = false -> if twitches 3 times
in your case once
well, the second option is poorly readable
if there are a lot of switch investments to help

Answer 3

The first option, sooner or later, can turn into:

Answer 4

Answer option Ivan Sokolov

if (!isRequested()) {
 requestRegister(numberField.getText().toString());
 return;
}

if (rulesField.isChecked()) {
    register(numberField.getText().toString(), codeField.getText().toString());
} else {
    showMessage(R.string.error_rules);
}