Thread: How can I improve this code?

Code:

killer.isAtWild()

Depending on where this code is, this shouldn't be necessary as long as it's handled by the controller.

Code:

killer.getEp() > 0 && getRiskedWealth(killer) > 76000 && getRiskedWealth(player) > 76000

It's not that bad, though I'd use constants to define the 76000 so it's easily changeable.

If it annoys you, you can make a name for it by making a seperate boolean and naming it as what all those checks do.

Code:

private boolean can76k(Player killer, Player player) {
	return killer.isAtWild() && killer.getEp() > 0 && getRiskedWealth(killer) > 76000 && getRiskedWealth(player) > 76000;
}

...

if (can76k(killer, player)) {
	...
}

In regards to:

Originally Posted by Skrew

It feels like there is a lot of &&'s and doesn't check all of them.

Then I believe what you're looking for is using the 'bitwise AND'.
If your code requires all the methods to be run then I suggest using a 'bitwise AND', the symbol of which is & (a single ampersand).
This is because using the regular AND (&&) symbol requires all values to be true/1, and as soon as it encounters a value which is not true/1 (i.e. false or 0) then it will not check the rest (as not all of the values will be true/1).
Using a bitwise AND checks each individual value regardless of if any of them are false/0.

The code would then be written as such:

Code:

if (killer.isAtWild() & killer.getEp() > 0 & getRiskedWealth(killer) > 76000 & getRiskedWealth(player) > 76000) {

Originally Posted by Tesla

In regards to:

Then I believe what you're looking for is using the 'bitwise AND'.
If your code requires all the methods to be run then I suggest using a 'bitwise AND', the symbol of which is & (a single ampersand).
This is because using the regular AND (&&) symbol requires all values to be true/1, and as soon as it encounters a value which is not true/1 (i.e. false or 0) then it will not check the rest (as not all of the values will be true/1).
Using a bitwise AND checks each individual value regardless of if any of them are false/0.

The code would then be written as such:

Code:

if (killer.isAtWild() & killer.getEp() > 0 & getRiskedWealth(killer) > 76000 & getRiskedWealth(player) > 76000) {

... and would not function as intended. This is the definitely not the appropriate operator to use, lol - may as well use logical or (||) in that case.

Also, to nitpick @ Clem and Arham, you'd probably want a combination of both of your solutions.

Code:

private static final int MIN_WEALTH = 76_000;

...

private boolean can(Player killer, player victim, int amount) {
...
}

...

public boolean can(Player killer, Player victim) {
return can(killer, victim, MIN_WEALTH);
}

...

if (can(killer, victim)) {
...
}

Originally Posted by hc747

... and would not function as intended. This is the definitely not the appropriate operator to use, lol - may as well use logical or (||) in that case.

Explain
Whether or not that would be the best way to go about solving the problem is another topic of debate, however the operator would certainly function as intended.

Originally Posted by Tesla

In regards to:

Then I believe what you're looking for is using the 'bitwise AND'.
If your code requires all the methods to be run then I suggest using a 'bitwise AND', the symbol of which is & (a single ampersand).
This is because using the regular AND (&&) symbol requires all values to be true/1, and as soon as it encounters a value which is not true/1 (i.e. false or 0) then it will not check the rest (as not all of the values will be true/1).
Using a bitwise AND checks each individual value regardless of if any of them are false/0.

The code would then be written as such:

Code:

if (killer.isAtWild() & killer.getEp() > 0 & getRiskedWealth(killer) > 76000 & getRiskedWealth(player) > 76000) {

There's probably 1 case in a million where using something like this actually makes sense. If your condition requires all the methods to be called, then it's because you've messed up somewhere. There's a reason you never see stuff like that...

Originally Posted by Tesla

Explain
Whether or not that would be the best way to go about solving the problem is another topic of debate, however the operator would certainly function as intended.

It works, but it's inefficient and just proves your code is bad.

Originally Posted by Tesla

Explain
Whether or not that would be the best way to go about solving the problem is another topic of debate, however the operator would certainly function as intended.

No, it'd still achieve the same evaluation, (false & true) is equal to (false && true), just in a less efficient manner - the first evaluates both conditions, whereas the second ceases evaluation after reaching false. If as you state, 'the code requires all methods to be run', then they may as well not be used as a branch condition, but rather, just executed.

Originally Posted by Vincent

It kind of is what OP asked though and reduces the amount of &&s. lol.

Most likely a bug, one of the 4 conditions being true/false when it shouldn't. It's not by calling each condition that it will solve the issue (in this situation) since && ultimately returns the same results as &.