Looking for some constructive criticism

Discussion in 'Plugin Development' started by d3x, Feb 3, 2011.

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

    d3x

    Today is my first time using Java. I just installed the JDK for the first time in my life about 6 hours ago! I dont want to pick up any bad habits so could some people go over my code quickly and point out flaws/bad habits?

    Code:
    //WebInventory.java
    package com.bukkit.d3x.WebInventory;
    
    import java.io.*;
    import java.util.HashMap;
    
    import org.bukkit.Server;
    import org.bukkit.entity.Player;
    import org.bukkit.plugin.PluginDescriptionFile;
    import org.bukkit.plugin.PluginLoader;
    import org.bukkit.plugin.java.JavaPlugin;
    
    /**
     * WebInventory for Bukkit
     *
     * @author d3x
     */
    public class WebInventory extends JavaPlugin {
        private final HashMap<Player, Boolean> debugees = new HashMap<Player, Boolean>();
        public WebInventory(PluginLoader pluginLoader, Server instance,
                PluginDescriptionFile desc, File folder, File plugin,
                ClassLoader cLoader) throws IOException {
            super(pluginLoader, instance, desc, folder, plugin, cLoader);
        }
        public void onEnable() {
            PluginDescriptionFile pdfFile = this.getDescription();
            System.out.println( pdfFile.getName() + " version " + pdfFile.getVersion() + " is enabled!" );
            new WebServer(this);
        }
        public void onDisable() {
            System.out.println("Goodbye world!");
        }
        public boolean isDebugging(final Player player) {
            if (debugees.containsKey(player)) {
                return debugees.get(player);
            } else {
                return false;
            }
        }
        public void setDebugging(final Player player, final boolean value) {
            debugees.put(player, value);
        }
    }
    Code:
    //WebServer.java
    package com.bukkit.d3x.WebInventory;
    
    import java.net.*;
    import java.io.*;
    
    import org.bukkit.inventory.ItemStack;
    import org.bukkit.inventory.PlayerInventory;
    
    public class WebServer extends Thread
    {
        int WebServerMode;
        WebInventory WebInventoryInstance;
        Socket WebServerSocket;
        public WebServer(WebInventory i)
        {
            WebServerMode = 0;
            WebInventoryInstance = i;
            start();
        }
        public WebServer(WebInventory i, Socket s)
        {
            WebServerMode = 1;
            WebInventoryInstance = i;
            WebServerSocket = s;
            start();
        }
        public void run()
        {
            try
            {
            if ( WebServerMode == 0 )
            {
                ServerSocket s = new ServerSocket(25555);
                for (;;)
                    new WebServer(WebInventoryInstance, s.accept());
            } else {
                BufferedReader in = new BufferedReader(new InputStreamReader(WebServerSocket.getInputStream()));
                DataOutputStream out = new DataOutputStream(WebServerSocket.getOutputStream());
                try
                {
                    String l, g, json;
                    while ( (l = in.readLine()).length() > 0 )
                    {
                        //System.out.println("Line: '" + l + "'\n");
                        if ( l.startsWith("GET") )
                        {
                            g = (l.split(" "))[1];
                            System.out.println("[WebInventoryWebServer] Client reqested '" + g + "'.");
                            if ( g.equals("/json/user_inventories") )
                            {
                                //System.out.println("!/json/user_inventories");
                                if ( WebInventoryInstance.getServer().getOnlinePlayers().length < 1 )
                                {
                                    json = "{user_inventories: []}\n";
                                } else {
                                    json = "{user_inventories: [\n";
                                    for ( org.bukkit.entity.Player p : WebInventoryInstance.getServer().getOnlinePlayers() )
                                    {
                                        json = json + "    {user: '" + p.getDisplayName() + "', inventory: [\n";
                                        PlayerInventory pi = WebInventoryInstance.getServer().getPlayer(p.getDisplayName()).getInventory();
                                        ItemStack[] items = pi.getContents();
                                        for ( int i = 0; i <= 35; i++ )
                                            json = json + "        {type: " + items[i].getTypeId() + ", amount: " + items[i].getAmount() + "}\n";
                                        json = json + "    ]}\n";
                                    }
                                    json = json + "]}";
                                }
                                //System.out.println("[WebInventoryWebServer] " + json);
                                out.writeBytes("HTTP/1.1 200 OK\r\n");
                                //out.writeBytes("Content-Type: application/json\r\n");
                                out.writeBytes("Content-Type: text/html\r\n");
                                out.writeBytes("Pragma: no-cache\r\n");
                                out.writeBytes("Cache-Control: no-cache\r\n");
                                out.writeBytes("Content-Length: " + json.length() + "\r\n");
                                out.writeBytes("Server: d3x's shitty WebServer class!\r\n");
                                out.writeBytes("Connection: Close\r\n\r\n");
                                out.writeBytes(json);
                            }
                        }
                    }
                }
                catch (Exception e) { System.out.println("[WebInventoryWebServer]" + e); }
                out.close();
            }
            }
            catch (Exception e){ System.out.println("[WebInventoryWebServer]" + e); }
        }
    }
    It's a backend for a html/javascript frontend to disaplay online user's inventories. I hope to soon expand it to probe LWC for chest locations and show contents too.
     
  2. Offline

    Plague

    I'm not a java programmer myself, but a long time C system programmer, so I can only have programming hints...
    Just two actually, do not use "Goodbye world" exit message, add at least a name, I have 5 goodbyes on my server and what does that give me?
    The code looks ok, just your indentation on some places is missing, but that's all.
     
  3. Offline

    d3x

    Thanks for looking at it. I too am here from C.

    I was not going to keep any messages like that in for the actual release. Also some of those ifs are temporary so I didn't indent the code.
     
  4. Offline

    eisental

    Very nice idea,
    It looks pretty well written.

    I think you need to interrupt the thread in onDisable() to actually stop the webserver when the plugin is disabled. It's mostly called on server shutdown but not always. For that you need to make a class field for the WebServer object instead of the anonymous one you have. I would also call the Thread.start() method from onEnable():
    For example
    server = new WebServer(this);
    server.start();

    and in onDisable():
    server.interrupt();

    you then need to update your try/catch in the thread to specifically catch ThreadInterruptedException to kill it gracefully.

    If you want to be really tidy you can use the Logger class for log messages:
    add a logger field
    private static final Logger l = Logger.getLogger("Minecraft");
    then you use
    l.info(), l.warning(), etc.

     
  5. Offline

    d3x

    Thanks for the feedback. I have done some changes and would once again appreciate any input.

    WebInventory.java
    WebServer.java
    WebServerClient.java

    Also I have been using Notepad2 to write this, does anyone know of a lightweight IDE with just syntax highlight and autocomplete?
     
Thread Status:
Not open for further replies.

Share This Page