3

I'm a beginner level programmer who's just starting to work on actual projects, and I'm starting to think about things such as efficiency and if my code looks professional. I was wondering if, when trying to check multiple booleans, is it better to use nested if statements, or multiple && and || operators.

Action action = event.getAction();
Material holding = event.getItem().getType();
if((action.equals(Action.RIGHT_CLICK_AIR)||(action.equals(Action.RIGHT_CLICK_BLOCK))))
{
    if((event.hasItem())&&(holding.equals(Material.COMPASS)))
    {
        //if the player right clicked while holding a compass
    }
}

Does this look right? I tried to group the like if-statements together. Also, if there's anything else I can do to improve my formatting, please tell me! Thanks.

Pshemo
  • 122,468
  • 25
  • 185
  • 269
  • And for me such code looks little disturbing. Having *only* nested `if` makes it look like there should be something before or after it, like maybe someone forgot to add `else{..}` part, so it should consult author about it. `if(conditionA && condigionB)` doesn't have that problem. – Pshemo Jul 08 '20 at 20:04

5 Answers5

2

Welcome to the Stack Overflow community!

There is no problem with the code shared in the question. In some cases, it is better to opt for legibility so that your co-workers will be able to understand the proposed code better. But in the end, this is very subjective.

IMHO, it is easier to understand if we write all the conditions at once. So,

Action action = event.getAction();
Material holding = event.getItem().getType();

Boolean isRequiredAction = action.equals(Action.RIGHT_CLICK_AIR) || action.equals(Action.RIGHT_CLICK_BLOCK)

if (
  isRequiredAction
  && event.hasItem() 
  && holding.equals(Material.COMPASS)
)
{
  // logic...
}

However, if you really want advice and tips on how to refactor it and best practices in a particular language, try Code Review community.

1

imo for a personal taste, i would put those nested conditions in a boolean variable that can explain the behavior as much as the comment you let in the block, like:

boolean isActionRightClick = action.equals(Action.RIGHT_CLICK_AIR ||action.equals(Action.RIGHT_CLICK_BLOCK);

boolean isHoldingACompass = event.hasItem() && holding.equals(Material.COMPASS);

and then

if ( isActionRightClick && isHoldingACompass ) {...}
0

Yes your code looks very good to me. I used to work on big projects and uses nested if statements, or multiple && and || operators which saves time. In your code efficiency can be traced at :

if((action.equals(Action.RIGHT_CLICK_AIR)||(action.equals(Action.RIGHT_CLICK_BLOCK))))

As now check only one condition in the or statement will satisfy the if condition which will save time and also shorten the code length.

You can make this code more shorter by removing unwanted parenthesis from your code. Which you must take care in future.

For more details related to efficient coding you can visit this link: https://docs.oracle.com/cd/E80738_01/pt854pbh2/eng/pt/tpcd/task_WritingMoreEfficientCode-0749ba.html#topofpage

Sumit Singh
  • 487
  • 5
  • 19
0

I think you could make it even more shorter by using ternary operator (?:) right.

if (expression1) {
    result = 1;
} else if (expression2) {
    result = 2;
} else if (expression3) {
    result = 3;
} else {
    result = 0;
}
result = (expression1) ? 1 : (expression2) ? 2 : (expression3) ? 3 : 0;
gunr2171
  • 16,104
  • 25
  • 61
  • 88
0

This is good to think about the quality/readability of your code.

Nested "if" are a good question in most of the case i think this depends of people. Some people prefer to nest it, to evaluate condition one after another. Some other prefer to not nest for not lose the track in the block.

But in most of the case be careful to not do to much if statement and try to replace it with pattern design (Easier said than done.). You can find a lot of it in java-design-patterns

Ruokki
  • 869
  • 1
  • 7
  • 24