Solved Any way to shorten nested IFs?

Discussion in 'Plugin Development' started by octoshrimpy, Aug 28, 2014.

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

    octoshrimpy

    Trying to maximize efficiency, and minimize code, I figured there's probably a better way to do things than nested IFs. any way of compacting the code below?
    notice: it's just an example of checks, for learning purposes only.

    Code:java
    1. public boolean onCommand(CommandSender s, Command cmd, String label, String[] args){
    2. if(cmd.getName().equalsIgnoreCase("f")){
    3. if(!(s instanceof Player)){
    4. s.sendMessage("cannot run this specific command from console.");
    5. //vs
    6. this.getLogger().info("cannot run this specific command from console");
    7. }else{
    8. if(!(args.length == 0)){
    9. s.sendMessage("this is a non-descriptive error message because you used arguments");
    10. s.sendMessage(" -O");
    11. }else{
    12. //do something
    13. }
    14. }
    15. }
    16. return true;
    17. }


    also, which is better? to log an info or warn to console after a s instanceofPlayer check, or still use s.sendMessage();?
     
  2. Offline

    Necrodoom

    octoshrimpy Remember that there are more senders than console and player. Commandblocks cannot view sendMessage being sent back to them, so youd may want to output the commandblock location to log, so you know whats wrong. In this case, logger is better.

    Also, you should learn how to use else-if, as opposed to nested if elses. Returning on invalid command can also help cutting down on nesting.
     
  3. Offline

    octoshrimpy

    Necrodoom I use else if, specially on CommandHandlers, which end up being big. I've seen things such as:

    if(condition)
    //do something

    without any brackets, or anything, and it still works. What is the difference/functionality?
    I didn't else-if up there because cmd.getName() and args.length don't have anything in common, and it was a nice way to keep track of code, but you're right, could have been cut down. also, I will remember to add a check for commandblocks.
     
  4. Offline

    TheOddPuff

    Using 'return' in a method will stop the code on that point

    Code:java
    1. public boolean onCommand(CommandSender s, Command cmd, String label, String[] args) {
    2. if(!cmd.getName().equalsIgnoreCase("f")) {
    3. return false;
    4. }
    5. if(!(s instanceof Player)){
    6. s.sendMessage("cannot run this specific command from console.");
    7. //vs
    8. this.getLogger().info("cannot run this specific command from console");
    9. return true;
    10. }
    11. if(args.length > 0) {
    12. s.sendMessage("this is a non-descriptive error message because you used arguments");
    13. s.sendMessage(" -O");
    14. } else {
    15. //do something
    16. }
    17. }
     
  5. Offline

    coldandtired

    If you're using Java 7+ you can switch on Strings ("f" is a terrible name for a command, by the way)
    Code:java
    1. switch (cmd.getName().toUpperCase())
    2. {
    3. case ("F"):
    4. blah;
    5. case ("G"):
    6. blah;
    7. }

    Within the block you can return to avoid elses
    Code:java
    1. switch (cmd.getName().toUpperCase())
    2. {
    3. case ("F"):
    4. if (!(sender instanceof Player))
    5. {
    6. System.out.println("Command needs a player!");
    7. return true;
    8. }
    9. Player p = (Player)sender;
    10. ...
    11. case ("G"):
    12. blah;
    13. }
     
  6. Offline

    Necrodoom

    octoshrimpy No brackets means just one line of code in the code block.
     
  7. Offline

    octoshrimpy

    TheOddPuff that looks better, I'll give it a try! thank you!

    coldandtired
    :) Also, too bad bukkit has to be compiled for Java6... I'll definitely look up case and switch, though! #LearnAllTheThings

    Necrodoom Got it, will keep it in mind. :)
     
  8. Offline

    Konkz

    wut
     
  9. Offline

    LucasEmanuel

    octoshrimpy
    It doens't, I'm working against Java 8 just fine. :)
     
  10. Offline

    Necrodoom

    Konkz LucasEmanuel per bukkit proper practice you should compile against Java 6 to ensure that all servers are able to run your plugin.
     
  11. Also, you can indent like that.

    Code:java
    1. if (something)
    2. {
    3. do this
    4. }
    5. else
    6. {
    7. do something else.
    8. }
     
  12. Offline

    LucasEmanuel

    Taking a look at mcstats.org only about 2% of all servers are using Java 6, 90% are using Java 7, and 8% are using Java 8. So you can work against Java 7 just fine, it is also a lot nicer to work against than Java 6. :)
     
  13. Offline

    Necrodoom

    LucasEmanuel mcstats is dead.. Also, blocking servers from using your plugin not good practice.
     
  14. Offline

    LucasEmanuel

    What makes you think that? :)
     
  15. Offline

    Necrodoom

    LucasEmanuel the fact that the maintainer is gone?
    The site also manages to list non-plugins.
     
  16. Offline

    LucasEmanuel

    Oh, I guess my time at the university has consumed more time than I thought. Too bad to hear about that, I really liked it. Also your tone is unnecessarily rude, I'm just trying to have a friendly conversation.

    Back on topic:
    I would still go with switch-statements and Java 7. :)
     
  17. Offline

    Zupsub

    Don't worry about performance when you're working with ifs. Look on the readableness instead.
    Can be shorten to
    Code:java
    1. if (args.length != 0) {



    Might be nitpicking, but it doesn't really depend on lines. It will only execute the next statement, which can be more lines, so the following would be totally correct:
    Code:java
    1.  
    2. if (condition)
    3. while(sth)
    4. if (something)
    5. break;
    6. else
    7. blub();
    8. else
    9. if (condition2)
    10. switch (aString) {
    11. case "hallo": doSth(); break;
    12. case "nope": doSthOther(); break;
    13. default: break;
    14. }
    15. else
    16. doSthOther();


    Just to notice: Don't forget to insert neested breaks.
     
    Msrules123, octoshrimpy and Necrodoom like this.
  18. Offline

    user_90854156

    PHP:
    public boolean onCommand(CommandSender sCommand cmdString labelString[] args){
            if(
    cmd.getName().equalsIgnoreCase("f")){ //you shouldn't need this if you use multiple command classes
                
    if(instanceof Players.sendMessage(args.length != "you used arguments and shit" "cool message");
                
    this.getLogger().info("cannot run this specific command from console");
            }
            return 
    false;
      }
    I guess this is a shortened version of your code, but you should return false/true;
     
  19. Offline

    teej107

    Did you also know that 74% of statistics are made up on the spot?
     
    Msrules123 and octoshrimpy like this.
  20. Offline

    Syd

    MrTang
    This might be "shorter", but it isn't readable anymore.

    octoshrimpy
    I would recommend you to get away with "cmd.getName()" calls. It's way cleaner to create an own executor for every command instead of one big method.
    Also, to reduce nested if's, I usually return when a condition is false instead of executing code if a condition is true.
    It keeps your code on one level, making is easier to read and to debug.

    I'd also prefer to always use the sender to return a message, as that's the way the it should work. Because as it's an API, it doesn't require the sender to be the console if it's not a player.
    A plugin could add an IRC bot CommandSender for example, that would miss the message send to it, when using the logger.

    About the Java 6 vs Java 7 dispute:
    I honestly don't understand anyone who runs Java 6 outside of an specialized enviroment (-> things that REQUIRE Java 6) anymore. Version 6 reached EOL 1,5 years ago, so running this on a server, accessable via internet, means that the owner wants to be hacked.

    However, that Bukkit compiles to Java 6 is related to the fact that Mojang does. And they do, as they have to keep the client in mind, which is also used by users and not by admins or developers.

    I for myself keep my public plugins with Java 6, while I do my private plugin in Java 7. But I hope Mojang raises the limit to Java 7 soon.

    (Also, never trust a statistic you havn't forged yourself. ;))
     
  21. Offline

    octoshrimpy

    Necrodoom I knew I had read about Java6 somewhere. Thank you for confirming it.

    Zupsub I totally forgot the nested break;, which was what confused me that I wasn't able to stick an else and the end. xD Now I know, thank you!

    MrTang
    Code:
    //you shouldn't need this if you use multiple command classes
    Being new to bukkit, and Java, how would one go about doing that? having a class for each command that can be called, and then calling it everytime the command is ran?

    Again, still new to the lingo. What would an executor look like?
     
  22. Offline

    Syd

    octoshrimpy

    Add a line getCommand("yourcommandname").setExecutor(new YourCommandExecutor()); in your onEnable code.
    The YourCommandExecutor class has to implement the CommandExecutor Interface, that defines that your class needs to have an onCommand method, like the one you're currently using.
     
  23. Offline

    octoshrimpy

    Syd something along the lines of this?
    Code:java
    1.  
    2. public class Main extends JavaPlugin implements Listener{
    3.  
    4. public void onEnable(){
    5. getServer().getPluginManager().registerEvents(this, this);
    6. getCommand("beginnerCommand").setExecutor(new cmdExecutor());
    7. }
    8. }
    9.  
    10. public class cmdExecutor implements CommandExecutor{
    11.  
    12. public boolean onCmd(CommandSender s, Command cmd, String label, String[] args){
    13. if(args.length==0)
    14. s.sendMessage("generic help message");
    15. return true;
    16. if(args.length==1)
    17. if(args[0].equalsIgnoreCase("hola"))
    18. s.sendMessage("heya there.");
    19. return true;
    20. if(args[0].equalsIgnoreCase("howAboutNo"))
    21. s.sendMessage("alrighty, then");
    22. return true;
    23. }
    24. }
    25.  


    threw both classes in the same paste, but they would be different classes, obviously.
     
  24. Offline

    TheHandfish

    You aren't checking to see if the proper command is being used. Toss an if statement in there that detects whether or not cmd.getName().equalsIgnoreCase("beginnerCommand").

    NOTE: Don't forget to list your command in your plugin.yml.
     
  25. Offline

    octoshrimpy

    What I currently have is this: a CommandHandler class, and an onCommand on my main, which passes the arguments to the CommandHandler class. This CommandExecutor feels better. More natural, and definitely cleaner.

    --EDIT-- this was sent before TheHandfish 's message above. ~Ninja'd~
     
  26. Offline

    TheHandfish

    Regardless, you should put the boolean I told you about in there.
     
  27. Offline

    octoshrimpy

    TheHandfish isn't that what
    Code:java
    1. getCommand("beginnerCommand").setExecutor(new cmdExecutor());
    is for? so it will only send stuff to the executor if the initial command is "beginnerCommand" ?
     
  28. Offline

    Syd

    octoshrimpy
    Setting the executor should work, however your onCommand methods misses some brackets.
    The if without { } is only for the next statement, if you want multiple statements you have to use { }.
    It would cause your code to exit in line 15 in all cases.

    You IDE should tell you that actually. ;)

    Also, against what TheHandfish, it is not needed to check if the proper command is used when you registered it this way.
    IMO any check if the passed command is the proper one is a sign of bad design.
     
  29. Offline

    TheHandfish

  30. Offline

    octoshrimpy

    Syd I appended it to the end of a working project I currently have up, the IDE didn't tell me it was wrong, mostly because it's probably wondering why there are 3 "classes" in one class :D
    if(){return true}
    if(){return true}
    is correct, though? And would I need to create a different executor for each main command? say, /test, /dig, /paranoia ?
     
Thread Status:
Not open for further replies.

Share This Page