Solved How good is my code?

Discussion in 'Plugin Development' started by Orange Tabby, Oct 3, 2015.

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

    Orange Tabby

    Hi, I am making a report plugin and I want to know if it's even good code before it is used on a server with more than 50 players, can one of you amazing people of the bukkit community read over and tell me if I picket up any bad coding habits or if I have methods that will cause lag, Thank you have some cake! [cake]

    Report Class (open)

    This is the report command class handles the saving and sending of the report
    Code:
    package com.skylordjay_.chat.user.commands;
    
    import java.io.File;
    import java.io.IOException;
    
    import org.bukkit.Bukkit;
    import org.bukkit.ChatColor;
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandExecutor;
    import org.bukkit.command.CommandSender;
    import org.bukkit.configuration.file.FileConfiguration;
    import org.bukkit.configuration.file.YamlConfiguration;
    import org.bukkit.entity.Player;
    
    import com.skylordjay_.chat.Chat;
    import com.skylordjay_.chat.admin.events.ReportNotify;
    
    public class Report implements CommandExecutor {
    
        File reportsFile;
        FileConfiguration reports;
        private ReportNotify reportNotify;
    
        public Report(Chat chat, ReportNotify reportNotify) {
            chat.getCommand("report").setExecutor(this);
            reportsFile = new File(chat.getDataFolder(), "reports.yml");
            reports = YamlConfiguration.loadConfiguration(reportsFile);
            saveReports();
            this.reportNotify = reportNotify;
        }
    
        public boolean onCommand(CommandSender sender, Command cmd,
                String commandLabel, String[] args) {
            if (args.length > 0) {
                Player targetPlayer = Bukkit.getServer().getPlayer(args[0]);
                if (targetPlayer != null) {
                    if (args.length > 1) {
                        String reason = avoidWordFromList(args, 0);
                        int id = reports.getKeys(false).size() + 1;
                        String formatedReason = formatReportReason(targetPlayer, sender, reason);
                        reports.set("ID: " + id, formatedReason);
                        saveReports();
                        reportNotify.send(formatedReason);
                        sender.sendMessage(ChatColor.AQUA + "The Report has been logged, all online admins have been notified!");
                        return false;
                    } else {
                        sender.sendMessage(ChatColor.RED + "Please give a Reason!");
                    }
                } else {
                    sender.sendMessage(ChatColor.RED + "§l" + args[0] + ChatColor.RED + " was not found!");
                }
            } else {
                sender.sendMessage(ChatColor.RED + "/Report [PlayerName] [Reason]");
            }
            return false;
        }
    
        public String formatReportReason(Player reported, CommandSender sender,
                String reason) {
            String reasonFormated = reported.getName() + " was reported by "
                    + sender.getName() + " for reason: " + reason;
            return reasonFormated;
        }
    
        public String avoidWordFromList(String[] list, int avoid) {
            String sentence = "";
            for (String words : list) {
                if (words != list[avoid]) {
                    sentence = sentence + words + " ";
                }
            }
            return sentence;
        }
    
        public void saveReports() {
            try {
                reports.save(reportsFile);
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
    


    ReportNotify Class (open)

    The report notify class is home of a method that sends the report reason to all the admins in the list
    Code:
    package com.skylordjay_.chat.admin.events;
    
    import java.util.ArrayList;
    import java.util.List;
    
    import org.bukkit.ChatColor;
    import org.bukkit.command.CommandSender;
    
    import com.skylordjay_.chat.Chat;
    
    public class ReportNotify {
    
        List<CommandSender> admin;
    
        public ReportNotify(Chat chat) {
            admin = new ArrayList<CommandSender>();
        }
    
        public void send(String message) {
            String formatedMessage = ChatColor.WHITE + "<" + ChatColor.AQUA
                    + "§lReport" + ChatColor.WHITE + "> " + ChatColor.DARK_AQUA
                    + message;
            for (CommandSender admins : admin) {
                admins.sendMessage(formatedMessage);
            }
        }
    
        public List<CommandSender> getAdmins() {
            return admin;
        }
    
    }
    


    ReportNotifyBoolean (open)

    ReportNotifyBoolean class is a command for admins to enable or disable the receiving of report notifications
    Code:
    package com.skylordjay_.chat.admin.commands;
    
    import java.util.List;
    
    import org.bukkit.ChatColor;
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandExecutor;
    import org.bukkit.command.CommandSender;
    
    import com.skylordjay_.chat.Chat;
    import com.skylordjay_.chat.admin.events.ReportNotify;
    
    public class ReportNotifyBoolean implements CommandExecutor {
    
        private ReportNotify reportnotify;
    
        public ReportNotifyBoolean(Chat chat, ReportNotify reportnotify) {
            chat.getCommand("ReportNotifications").setExecutor(this);
            this.reportnotify = reportnotify;
        }
    
        public boolean onCommand(CommandSender sender, Command cmd,
                String commandLabel, String[] args) {
            if (sender.hasPermission("chat.admin.commands.reportnotify")) {
                List<CommandSender> admins = reportnotify.getAdmins();
                if (!admins.contains(sender)) {
                    admins.add(sender);
                    sender.sendMessage(ChatColor.AQUA
                            + "Enabled Report Notifications!");
                } else {
                    admins.remove(sender);
                    sender.sendMessage(ChatColor.AQUA
                            + "Disabled Report Notifications!");
                }
            } else {
                sender.sendMessage(ChatColor.RED + "You can't use this command!");
            }
            return false;
        }
    
    }
    


    Loading Class (open)

    This class is called in the main onEnable method to load all the commands and events
    Code:
        public Manager(Chat chat) {
            loadReportTree(chat);
        }
    
        public void loadReportTree(Chat chat) {
            ReportNotify reportNotify = new ReportNotify(chat);
            new ReportNotifyBoolean(chat, reportNotify);
            new Report(chat, reportNotify);
        }
     
    Last edited: Oct 3, 2015
  2. Offline

    Gonmarte

    @Orange Tabby 2 words -> Plugin Development!
    This forum is not to ask if the code is good... If you have any troubles in your plugin let us know if you just want to know "HOW GOOD YOUR CODE IS" why you dont check by your self and if u see anything that u dont have sure and share it with us. Im not going to read all the code.
     
  3. Offline

    mbaxter ʇıʞʞnq ɐ sɐɥ ı

    • You have some package-private, non-final variables
    • You just print the stack trace of a caught exception without explaining to the user what happened
    • You are directly storing the section symbol (the color code) in your Strings
      • Use the ChatColor class for that sort of thing

    That's just a few issues I saw. For a better way of sharing code, try GitHub.
     
  4. Offline

    teej107

    I don't see anything wrong with this. Reviewing code is all part of programming and belongs in this section if it's about Bukkit development which it is.

    @Orange Tabby I just skimmed over the code and didn't see anything too terrible apart from what mbaxter pointed out. I can look it over more thoroughly later. It might help to explain what the plugin is supposed to do and what each class's purpose is.

    EDIT: Also I think a Set would be more appropriate than a List for your admin Collection.
     
  5. Offline

    Orange Tabby

    @mbaxter @teej107 Thank you for replying I'll take a look that those thank you
    here's the cake [cake]
     
  6. Offline

    RoboticPlayer

    A) Change your package name, you don't own the domain skylordjay_.com. Change it to something such as your GitHub respository, your email or me.(yourname).(projectname)
    B) As @mbaxter said, it's best to avoid using this symbol: §
     
    Orange Tabby likes this.
Thread Status:
Not open for further replies.

Share This Page