Any criticism of this code?

Discussion in 'Spigot Plugin Development' started by Omnivion, May 19, 2015.

  1. Omnivion

    Patron

    So, I've been playing with streams a lot lately, and I thought I'd write something for live usage on one of my servers.

    Any thoughts on the way I went about doing this?:

    Code (Text):
        @Override
        public boolean onCommand(final CommandSender sender, final Command cmd, final String label, final String[] args)
        {
            sender.sendMessage
            (
                ChatColor.GREEN + "The average ping is: " +

                Bukkit.getOnlinePlayers()
                .stream()
                .mapToInt(playerPing -> ((CraftPlayer) playerPing).getHandle().ping)
                .filter(
                        ms -> ms > 0 &&
                                  ms < 1000
                       )
                .average().getAsDouble()

                + "ms"
            );
            return true;
        }
     
    #1 Omnivion, May 19, 2015
    Last edited: May 20, 2015
  2. @Omnivion what's up with that format, really xD.
     
    • Agree Agree x 6
  3. The formatting is foul and makes it very hard to read.
     
    • Agree Agree x 2
  4. Your lambda in the filter statement never returns false.

    Your formatting is...weird.

    Your code could benefit readability-wise from splitting up the map-statement into smaller map statements with single operations. Which would also enable you to use method references on some.

    The playerPing variable is awkwardly named.
     
  5. Why is everything final?

    Wouldn't it be better to check if the command is "ping" or whatever command you have?

    And, the ping method looks very dirty, long, and difficult to read. I would just define the int:

    Code (Java):
    public int getPing(Player p) {
    CraftPlayer cp = (CraftPlayer) p;
    EntityPlayer ep = cp.getHandle();
    return ep.ping;
    }
    And in the command, use it to get the online players ping, filter, average, whatever you want to do.
     
    • Like Like x 1
    • Useful Useful x 1
  6. Omnivion

    Patron

    When would I need it to? I'm not making this released plugin, so there's no real need for me to add in a million and a half safety checks, seeing as I'll be the only one using it :p

    Meh, the compiler is going to make it final anyways (at least in the above case). Mine as well write it with immutability if I don't want it changed!

    This isn't final implementation, I just copy/pasted this right out of my development environment.

    Things like the formatting will change, but I do use an Allman formatting style. Thanks for the suggestion @Arman.
     
  7. That kind of sounds like "I don't care if it's made with good practice or not, nobody else will be using it". True, but it never hurts.
     
  8. Omnivion

    Patron

    When did I say I didn't want it?

    I will say now though, I don't really want suggestions to my overall style. Things like personal code format are purely opinion based, and I find the readability of allman to be pretty prime. It's fatuous to tell me to just format the code differently.

    I do appreciate the suggestion of using a separate method for that one part though :)

    Good point, I was originally going to take a slightly different path while I was writing it, so that got mixed in there. Fixed :)
     
    • Friendly Friendly x 1
  9. Omnivion

    Patron

    Fair enough. The final implementation will use better practices~
     
    • Like Like x 1
    • Funny Funny x 1
  10. Go away :(

    On a more serious note:
    I think you might have missed my point. The lambda will never under any circumstances whatsoever return false. It will always, no matter how high or low the ping value is, return true. With this in mind, you can just remove the filter call altogether, and it would have no effect on the outcome of your code.
     
    • Like Like x 1
  11. eqx

    eqx

    Use Ctrl+Shift+F looks nicer to read other than that look good!
     
  12. Omnivion

    Patron

    I already said I use an allman code style.
     
  13. Omnivion

    Patron

    oops, got my ampersand confused with my pipe ):

    It doesn't help that I was using more than just those pipes

    [​IMG]

    I already said I use an allman code style.
     
  14. But you can simple format your code, post here and undo formatation. It will make everything more easy
     
  15. Omnivion

    Patron

    And sacrifice readability for saved space? .. why?
     
  16. You couldn't be more wrong here. All but the rare custom tweaked java compilers (and no an AOT compiler does not count) will simply ignore the micro-optimization created by setting a param final. What compilers actually do is leave the marker for JIT so that JIT can do what it pleases with the micro-optimization and instead the compiler just checks to make sure you're not breaking any rules of finality.

    And the phrase is Might as well, not Mine as well
     
  17. Omnivion

    Patron

    Eclipse -> Window -> Preferences -> Java -> Editor -> Save Actions -> Configure -> Code Style

    But that's neither here nor there.
     
  18. I think you quoted the wrong person :p Seeing as I made no comment to your code style.
    Since I used to be an Allmans person before switching to k&r due to easier readability on my phone, since its hard to read when you have extra lines sometimes. xd
     
  19. Omnivion

    Patron

    No, I was talking about the final thing.

    Fair enough though, the only good reason to switch from the Allman master-race!