Thread: How can I improve this code?

Page 1 of 2 12 LastLast
Results 1 to 10 of 11
  1. #1 How can I improve this code? 
    Registered Member
    Join Date
    May 2015
    Posts
    104
    Thanks given
    30
    Thanks received
    2
    Rep Power
    11
    This may seem like a silly request, but this line of code looked messy to me in the first place, but also isn't working.

    Code:
    if (killer.isAtWild() && killer.getEp() > 0 && getRiskedWealth(killer) > 76000 && getRiskedWealth(player) > 76000) {
    It feels like there is a lot of &&'s and doesn't check all of them.
    Reply With Quote  
     

  2. #2  
    Registered Member
    clem585's Avatar
    Join Date
    Sep 2013
    Posts
    3,161
    Thanks given
    453
    Thanks received
    547
    Rep Power
    107
    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.
    Project thread
    [Only registered and activated users can see links. ]
    Reply With Quote  
     

  3. Thankful user:


  4. #3  
    (Official) Thanksgiver

    arham 4's Avatar
    Join Date
    Jan 2013
    Age
    17
    Posts
    3,002
    Thanks given
    5,973
    Thanks received
    1,519
    Rep Power
    977
    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)) {
    	...
    }
    I support...
    Spoiler for big signature tbh:



    List of my work [Only registered and activated users can see links. ]!
    Reply With Quote  
     

  5. Thankful user:


  6. #4  
    Developer

    Tesla's Avatar
    Join Date
    Feb 2012
    Posts
    593
    Thanks given
    502
    Thanks received
    353
    Rep Power
    777
    In regards to:
    Quote Originally Posted by Skrew View Post
    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) {
    [Only registered and activated users can see links. ]
    Some interesting Runescape/RSPS related threads:
    [Only registered and activated users can see links. ]
    [Only registered and activated users can see links. ]
    [Only registered and activated users can see links. ]

    My projects:
    [Only registered and activated users can see links. ]
    Quote Originally Posted by blaxe View Post
    weird system, normal people using spawnconfig.cfg?
    Quote Originally Posted by Andys1814 View Post
    rune-server is a big circle jerk around the user "Graham."
    Reply With Quote  
     

  7. #5  
    Registered Member
    hc747's Avatar
    Join Date
    Dec 2013
    Age
    20
    Posts
    1,193
    Thanks given
    2,373
    Thanks received
    494
    Rep Power
    472
    Quote Originally Posted by Tesla View Post
    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)) {
    ...
    }
    [Only registered and activated users can see links. ]
    [Only registered and activated users can see links. ]
    Reply With Quote  
     

  8. Thankful user:


  9. #6  
    Developer

    Tesla's Avatar
    Join Date
    Feb 2012
    Posts
    593
    Thanks given
    502
    Thanks received
    353
    Rep Power
    777
    Quote Originally Posted by hc747 View Post
    ... 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.
    [Only registered and activated users can see links. ]
    Some interesting Runescape/RSPS related threads:
    [Only registered and activated users can see links. ]
    [Only registered and activated users can see links. ]
    [Only registered and activated users can see links. ]

    My projects:
    [Only registered and activated users can see links. ]
    Quote Originally Posted by blaxe View Post
    weird system, normal people using spawnconfig.cfg?
    Quote Originally Posted by Andys1814 View Post
    rune-server is a big circle jerk around the user "Graham."
    Reply With Quote  
     

  10. #7  
    Registered Member
    clem585's Avatar
    Join Date
    Sep 2013
    Posts
    3,161
    Thanks given
    453
    Thanks received
    547
    Rep Power
    107
    Quote Originally Posted by Tesla View Post
    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...

    Quote Originally Posted by Tesla View Post
    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.
    Project thread
    [Only registered and activated users can see links. ]
    Reply With Quote  
     

  11. Thankful user:


  12. #8  
    Registered Member

    Join Date
    Feb 2010
    Posts
    708
    Thanks given
    132
    Thanks received
    293
    Rep Power
    348
    Quote Originally Posted by clem585 View Post
    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...



    It works, but it's inefficient and just proves your code is bad.
    It kind of is what OP asked though and reduces the amount of &&s. lol.
    Reply With Quote  
     

  13. #9  
    Registered Member
    hc747's Avatar
    Join Date
    Dec 2013
    Age
    20
    Posts
    1,193
    Thanks given
    2,373
    Thanks received
    494
    Rep Power
    472
    Quote Originally Posted by Tesla View Post
    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.
    [Only registered and activated users can see links. ]
    [Only registered and activated users can see links. ]
    Reply With Quote  
     

  14. #10  
    Registered Member
    clem585's Avatar
    Join Date
    Sep 2013
    Posts
    3,161
    Thanks given
    453
    Thanks received
    547
    Rep Power
    107
    Quote Originally Posted by Vincent View Post
    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 &.
    Project thread
    [Only registered and activated users can see links. ]
    Reply With Quote  
     

Page 1 of 2 12 LastLast

Thread Information
Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Similar Threads

  1. How can I improve this PC?
    By Codiction in forum Hardware
    Replies: 4
    Last Post: 07-27-2017, 06:21 AM
  2. How can I improve this piece of code?
    By arham 4 in forum Help
    Replies: 2
    Last Post: 08-05-2015, 07:44 PM
  3. How can I change this code?
    By Mouldy in forum Help
    Replies: 14
    Last Post: 05-15-2014, 08:31 PM
  4. How can I improve this?
    By RuneWings317 in forum Graphics
    Replies: 16
    Last Post: 06-26-2012, 06:51 PM
  5. How can I improve this?
    By Colby in forum Application Development
    Replies: 15
    Last Post: 01-26-2010, 12:22 AM
Posting Permissions
  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •