multipel args whif color

Discussion in 'Plugin Development' started by Badwolf330, Oct 13, 2015.

Thread Status:
Not open for further replies.
  1. Offline

    Badwolf330

    Code:
                    if(args.length == 2) {
                    Main.sendActionBar(Bukkit.getPlayer(args[0]), args[1]);
                    }
                    if(args.length == 3) {
                    Main.sendActionBar(Bukkit.getPlayer(args[0]), args[1] + args[2]);
                    }
                    if(args.length == 4) {
                    Main.sendActionBar(Bukkit.getPlayer(args[0]), args[1] + args[2] + args[3]);
                    }
                    if(args.length == 5) {
                    Main.sendActionBar(Bukkit.getPlayer(args[0]), args[1] + args[2] + args[3] + args[4]);
                    }
                    if(args.length == 6) {
                    Main.sendActionBar(Bukkit.getPlayer(args[0]), args[1] + args[2] + args[3] + args[4] + args[5]);
                    }
                    if(args.length == 7) {
                    Main.sendActionBar(Bukkit.getPlayer(args[0]), args[1] + args[2] + args[3] + args[4] + args[5] + args[6]);
                    }
    
    how can i short this???
    and how can i give it a color is someone is using &3 ???
     
  2. Offline

    caseif

    This excerpt massively violates the principle of DRY (Don't Repeat Yourself) code. Alternatively, you can take advantage of a for-loop. It would look something like this:

    Code:
    Player player = Bukkit.getPlayer(args[0]); // get the player from the argument
    if (player == null) { // check that the player exists
        // do something
        return; // stop execution since the player is invalid
    }
    
    StringBuilder sb = new StringBuilder(); // create a new StringBuilder to construct your string
    for (int i = 1; i < args.length; i++) { // this will initialize i at 1, and increment it each loop by 1 until it reaches args.length
        sb.append(args); // append the current argument to the StringBuilder
    }
    String message = sb.toString(); // get the final string from the StringBuilder
    message = ChatColor.translateAlternateColorCodes(message, '&'); // translate the color codes (I might have swapped the arguments, so double-check this to be sure
    Main.sendActionBar(player, message); // finally, execute your method with the constructed message
    
    However (and I say this in the most polite way possible), the fact that you weren't using a for-loop to begin with is symptomatic of a lack of knowledge of basic Java. I would highly suggest reading Oracle's Java tutorial and getting the hang of the language with simpler projects before graduating to Bukkit plugins. :)
     
  3. Offline

    Badwolf330

    @caseif it isn't working :( it wil give me no error.....
     
  4. Offline

    boomboompower

    Umm its
    Code:
    ChatColor.translateAlternateColorCodes('&', message);
    The character comes first.
     
  5. Offline

    caseif

    Hence the comment next to it. :)
     
  6. Offline

    Scimiguy

    @caseif @Badwolf330
    Since he's checking exact values, it would be FAR more efficient to use a switch statement, not a for loop
     
  7. Offline

    caseif

    His implementation is flawed conceptually, though. His goal is to concatenate all arguments after the first into a single string, but checking the argument list length and manually creating the string is not a good nor efficient way to do it at all.
     
  8. Offline

    Scimiguy

    @caseif
    Say very much so, I was just correcting your statement ;)
     
  9. Offline

    DoggyCode™

    Haha a switch statement here? Like, how would you even do that?
    case 1: args[0]; break;
    case 2: args[1]; break;
    //and so on?

    That wouldn't be very efficient when you have the loops
     
  10. Offline

    Tecno_Wizard

    @DoggyCode™, close. Not a switch, but a loop.

    Code:
    if(args.lenght > 1) {
      String argTotal = "";
      for(int i = 1; i < args.lenght; i++) {
        argTotal += args[i];
      }
      Main.sendActionBar(Bukkit.getPlayer(args[0]), argTotal);
    }
     
  11. Offline

    DoggyCode™

    Huh? Is that a switch statement?
     
  12. Offline

    Tecno_Wizard

    @DoggyCode™, nope. Just a loop and some binary string adding.
     
  13. Offline

    DoggyCode™

    Yea... I replied to the guy saying that you could use a switch statement instead of a for loop and it would be much more efficient. Then I asked how can gave an example which I thought might could work (not efficient though)
     
  14. Offline

    Scimiguy

    @DoggyCode™

    Um. yeah?

    Code:
    switch (args.length) {
    case 1:
    }
    What's the problem?
     
  15. Offline

    Tecno_Wizard

  16. Offline

    DoggyCode™

    I know! That's what I thought, which is why I asked how you would use it and how it would be more efficient. Look at the quote which I included in my first reply
     
  17. Offline

    Tecno_Wizard

    @DoggyCode™, in fact, it's less efficient. The loop is better as far as efficiency.
     
  18. Offline

    mythbusterma

    @Tecno_Wizard @DoggyCode™

    I swear to god, the next person to say "efficiency".....

    Technically speaking, the switch statement is a few nanoseconds faster (i.e. there is literally no difference).

    The only thing that matters is readability, and I feel in most cases, the switch (or chained if-statements), would be the most readable for this.

    @Techno_Wizard

    I really don't know where you learned what was "more" or "less" efficient, but it was very wrong. How is a loop more efficient than chained comparisons that each lead to jump statements (the compile output of "switch")?

    It certainly is more code, but that doesn't make it slower.
     
    Zombie_Striker likes this.
  19. Offline

    Scimiguy

    @mythbusterma @Tecno_Wizard @DoggyCode™

    Exactly.

    Not only is it "more efficient", it's more readable, more adjustable, and does NOT violate DRY.

    Thanks for replying myth. I almost did it myself, but I decided it wasn't worth my time arguing the point
     
    mythbusterma likes this.
  20. Offline

    DoggyCode™

    But you would have to make like 20 cases..
     
  21. Offline

    mythbusterma

    @DoggyCode™

    I don't know why anyone would provide 20 arguments when typing a command?
     
  22. Offline

    Scimiguy

    @DoggyCode™

    More time spent coding = less time spent running.
    IF you have to put a bit of extra time in to make your plugin as good as possible, that's probably a good thing

    Think about it when running:
    If his args.length really is 20, then your loop has to run 20 times before it gets there.
    Switch statement jumps straight to it EVERY time
     
  23. I take it you're making some sort of broadcast plugin?

    '/broadcast hey there'.

    To capture the 'hey there' part only, just iterate through the args array starting from 1 (remember, args.length == 1 etc) and going up to args.length.
     
  24. Offline

    DoggyCode™

    Hmm, ok.

    /msg /broadcast?? If he wants to get all the args there's a reason

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Oct 29, 2015
  25. Offline

    Scimiguy

    @Badwolf330
    Regardless, you shouldn't need any of this, just use Arrays.toString();
     
  26. Offline

    caseif

    How did I know a debate would break out over this?

    Whether or not a switch statement is more efficient is entirely beside the point. It makes literally no sense whatsoever to check every possible argument count up to a limit when you have more advanced tactics like looping. As I said, it's a severe violation of the DRY principle, and furthermore, it's not extensible. Sure, you could include 20 if- or case-blocks, and it'll probably be good enough, but why even impose that limit when you can avoid it entirely while simultaneously using 10 times less code?

    And for the record, the non-loop approach is technically marginally faster than the loop approach. For the non-looping way, for an argument list of length n, the JVM does n integer comparison operations for length and n - 1 string concatentations. For the looping way, it does n + 1 integer comparisons, n - 1 string concatenations, and n integer addition operations. This being said, the performance loss caused by the loop approach is entirely negligible, especially in the face of the alternative's source code and limitations.
     
    Last edited: Oct 15, 2015
  27. Offline

    teej107

    Why are you still arguing over switch/for/if statements? Half of this thread seems offtopic to me. Using the StringUtils class and its method join(Object[], char, int, int) will cut down the code and make it easy to read. Readability should always go first before efficiency. The join method in fact uses a for-loop which you should use when dealing with args that could be of any length. Switch/if statements is the inappropriate way to go for this.
    @caseif's spoonfed code pretty much does the same thing as the join method.
     
  28. Offline

    caseif

    My intention wasn't to spoonfeed; I hate doing that. However, I was working on the assumption that for-loops were a new concept to OP, and couldn't think of a way to convey it other than by providing code demonstrating it. I did throw in a lot of comments to exactly describe what it was doing, though.
     
Thread Status:
Not open for further replies.

Share This Page