Feedback on java code for a custom message plugin

Status
This thread has been locked.
Harry

Harry

Rustacean
Management
Feedback score
10
Posts
1,606
Reactions
876
Resources
0
Hey guys. I've recently started developing in java and I just wanted someone to go through my code quickly for a custom message plugin quickly. It isn't completed at the moment so some things such as the hasPMEnabled function hasn't been done yet. If you have any feedback, please don't hesitate to reply with it below. Thanks everyone :)

Code: 
package me.whygopro.custommsg;

import java.util.HashMap;

import org.bukkit.entity.Player;
import org.bukkit.plugin.java.JavaPlugin;

import me.whygopro.custommsg.Commands.MessageCMD;
import me.whygopro.custommsg.Commands.ReplyCMD;

public class MainClass extends JavaPlugin{
   
    //public static HashMap<String, Integer> PMEnabledList = new HashMap<String, Integer>();
    public static HashMap<Player, Player> lastRecipient = new HashMap<Player, Player>();
   
    public void onEnable() {
        getLogger().info("has been enabled.");
       
        this.getCommand("message").setExecutor(new MessageCMD());
        this.getCommand("reply").setExecutor(new ReplyCMD());
    }
   
    public void onDisable() {
        getLogger().info("has been disabled.");
    }
   
    public void loadConfig() {
       
    }
}
Code: 
package me.whygopro.custommsg;

import org.bukkit.entity.Player;

import net.md_5.bungee.api.ChatColor;

public class MessageClass {
    public static void sendMessage(Player sender, Player receiver, String message) {
        if(isOnline(receiver) == false) {
            sender.sendMessage(ChatColor.RED + "That user is no longer online.");
        }
        else if(sender.getName().toString().equalsIgnoreCase(receiver.getName().toString())) {
            sender.sendMessage(ChatColor.RED + "You cannot send a private message to yourself.");
        }
        else if(hasPMEnabled(sender) == false) {
            sender.sendMessage(ChatColor.RED + "You currently have your private messages disabled.");
        }
        else if(hasPMEnabled(receiver) == false) {
            sender.sendMessage(ChatColor.RED + "That user currently has their private messages disabled.");
        }
        else {
            sender.sendMessage(ChatColor.GRAY + "[me -> " + receiver.getName().toString() + "] " + message.toString());
            receiver.sendMessage(ChatColor.GRAY + "[" + sender.getName().toString() + " -> me] " + message.toString());
           
            MainClass.lastRecipient.put(sender, receiver);
            MainClass.lastRecipient.put(receiver, sender);
        }
    }
   
    public static boolean hasPMEnabled(Player player) {
        return true;
    }
   
    public static boolean isOnline(Player player) {
        if(player == null) {
            return false;
        }
        else {
            return true;
        }
    }
   
    public static String getMessage(String[] args) {
        StringBuilder builder = new StringBuilder();

        for(int x = 0; x < args.length; x++)
           
            builder.append(args[x]).append(" ");
       
        return builder.toString();
    }
}
Code: 
package me.whygopro.custommsg.Commands;

import org.bukkit.Bukkit;
import org.bukkit.command.Command;
import org.bukkit.command.CommandExecutor;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;

import me.whygopro.custommsg.MessageClass;
import net.md_5.bungee.api.ChatColor;

public class MessageCMD implements CommandExecutor {
        public boolean onCommand(CommandSender player, Command cmd, String label, String[] args) {
            if (player instanceof Player == false) {
                player.sendMessage(ChatColor.RED + "You must be a player to send a message.");
            }
            else if(args.length == 0) {
                player.sendMessage(ChatColor.GREEN + "Please type the player's username followed by a message.");
                player.sendMessage(ChatColor.GREEN + "/msg <player> <message>");
            }
            else if(args.length == 1) {
                player.sendMessage(ChatColor.GREEN + "Please add a message after the player's name.");
                player.sendMessage(ChatColor.GREEN + "/msg <player> <message>");
            }
            else {
                Player sender = (Player) player;
                Player receiver = Bukkit.getPlayerExact(args[0]);
               
                    MessageClass.sendMessage(sender, receiver, MessageClass.getMessage(args));
                }
            return true;
        }
}
Code: 
package me.whygopro.custommsg.Commands;

import org.bukkit.command.Command;
import org.bukkit.command.CommandExecutor;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;

import me.whygopro.custommsg.MainClass;
import me.whygopro.custommsg.MessageClass;
import net.md_5.bungee.api.ChatColor;

public class ReplyCMD implements CommandExecutor {
        public boolean onCommand(CommandSender player, Command cmd, String label, String[] args) {
            if (player instanceof Player == false) {
                player.sendMessage(ChatColor.RED + "You must be a player to send a message.");
            }
            else if(args.length == 0) {
                player.sendMessage(ChatColor.GREEN + "Use /reply to message the user you last contacted..");
                player.sendMessage(ChatColor.GREEN + "/reply <message>");
            }
            else {
                Player sender = (Player) player;
               
                MessageClass.sendMessage(sender, MainClass.lastRecipient.get(sender) , MessageClass.getMessage(args));
                }
            return true;
        }
}
 
Click to expand...
PebbleHost
High performance, consistent uptime and fast support. Minecraft hosting that just works.
Ghast

Ghast

Founding Father of Hypocrisy - https://artemis.ac
Supreme
Feedback score
54
Posts
2,096
Reactions
3,285
Resources
79
WhyGoPro said:
Hey guys. I've recently started developing in java and I just wanted someone to go through my code quickly for a custom message plugin quickly. It isn't completed at the moment so some things such as the hasPMEnabled function hasn't been done yet. If you have any feedback, please don't hesitate to reply with it below. Thanks everyone :)

Code: 
package me.whygopro.custommsg;

import java.util.HashMap;

import org.bukkit.entity.Player;
import org.bukkit.plugin.java.JavaPlugin;

import me.whygopro.custommsg.Commands.MessageCMD;
import me.whygopro.custommsg.Commands.ReplyCMD;

public class MainClass extends JavaPlugin{
  
    //public static HashMap<String, Integer> PMEnabledList = new HashMap<String, Integer>();
    public static HashMap<Player, Player> lastRecipient = new HashMap<Player, Player>();
  
    public void onEnable() {
        getLogger().info("has been enabled.");
      
        this.getCommand("message").setExecutor(new MessageCMD());
        this.getCommand("reply").setExecutor(new ReplyCMD());
    }
  
    public void onDisable() {
        getLogger().info("has been disabled.");
    }
  
    public void loadConfig() {
      
    }
}
Code: 
package me.whygopro.custommsg;

import org.bukkit.entity.Player;

import net.md_5.bungee.api.ChatColor;

public class MessageClass {
    public static void sendMessage(Player sender, Player receiver, String message) {
        if(isOnline(receiver) == false) {
            sender.sendMessage(ChatColor.RED + "That user is no longer online.");
        }
        else if(sender.getName().toString().equalsIgnoreCase(receiver.getName().toString())) {
            sender.sendMessage(ChatColor.RED + "You cannot send a private message to yourself.");
        }
        else if(hasPMEnabled(sender) == false) {
            sender.sendMessage(ChatColor.RED + "You currently have your private messages disabled.");
        }
        else if(hasPMEnabled(receiver) == false) {
            sender.sendMessage(ChatColor.RED + "That user currently has their private messages disabled.");
        }
        else {
            sender.sendMessage(ChatColor.GRAY + "[me -> " + receiver.getName().toString() + "] " + message.toString());
            receiver.sendMessage(ChatColor.GRAY + "[" + sender.getName().toString() + " -> me] " + message.toString());
          
            MainClass.lastRecipient.put(sender, receiver);
            MainClass.lastRecipient.put(receiver, sender);
        }
    }
  
    public static boolean hasPMEnabled(Player player) {
        return true;
    }
  
    public static boolean isOnline(Player player) {
        if(player == null) {
            return false;
        }
        else {
            return true;
        }
    }
  
    public static String getMessage(String[] args) {
        StringBuilder builder = new StringBuilder();

        for(int x = 0; x < args.length; x++)
          
            builder.append(args[x]).append(" ");
      
        return builder.toString();
    }
}
Code: 
package me.whygopro.custommsg.Commands;

import org.bukkit.Bukkit;
import org.bukkit.command.Command;
import org.bukkit.command.CommandExecutor;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;

import me.whygopro.custommsg.MessageClass;
import net.md_5.bungee.api.ChatColor;

public class MessageCMD implements CommandExecutor {
        public boolean onCommand(CommandSender player, Command cmd, String label, String[] args) {
            if (player instanceof Player == false) {
                player.sendMessage(ChatColor.RED + "You must be a player to send a message.");
            }
            else if(args.length == 0) {
                player.sendMessage(ChatColor.GREEN + "Please type the player's username followed by a message.");
                player.sendMessage(ChatColor.GREEN + "/msg <player> <message>");
            }
            else if(args.length == 1) {
                player.sendMessage(ChatColor.GREEN + "Please add a message after the player's name.");
                player.sendMessage(ChatColor.GREEN + "/msg <player> <message>");
            }
            else {
                Player sender = (Player) player;
                Player receiver = Bukkit.getPlayerExact(args[0]);
              
                    MessageClass.sendMessage(sender, receiver, MessageClass.getMessage(args));
                }
            return true;
        }
}
Code: 
package me.whygopro.custommsg.Commands;

import org.bukkit.command.Command;
import org.bukkit.command.CommandExecutor;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;

import me.whygopro.custommsg.MainClass;
import me.whygopro.custommsg.MessageClass;
import net.md_5.bungee.api.ChatColor;

public class ReplyCMD implements CommandExecutor {
        public boolean onCommand(CommandSender player, Command cmd, String label, String[] args) {
            if (player instanceof Player == false) {
                player.sendMessage(ChatColor.RED + "You must be a player to send a message.");
            }
            else if(args.length == 0) {
                player.sendMessage(ChatColor.GREEN + "Use /reply to message the user you last contacted..");
                player.sendMessage(ChatColor.GREEN + "/reply <message>");
            }
            else {
                Player sender = (Player) player;
              
                MessageClass.sendMessage(sender, MainClass.lastRecipient.get(sender) , MessageClass.getMessage(args));
                }
            return true;
        }
}
Click to expand...
Instead of using the logger, use a print option, it is so much better.
 
Harry

Harry

Rustacean
Management
Feedback score
10
Posts
1,606
Reactions
876
Resources
0
Ghast said:
Instead of using the logger, use a print option, it is so much better.
Click to expand...
Thanks for the info. So you're suggesting System.out.println("")? Also, What would be the advantages of this or is this just a personal preference?
 
Last edited:
Ghast

Ghast

Founding Father of Hypocrisy - https://artemis.ac
Supreme
Feedback score
54
Posts
2,096
Reactions
3,285
Resources
79
WhyGoPro said:
Thanks for the info. So you're suggesting System.out.println("")? Also, What would be the advantages of this or is this just a personal preference?
Click to expand...
Personal preference, it is easier to read.
 
HippoPlayz

HippoPlayz

Feedback score
1
Posts
108
Reactions
30
Resources
0
I would avoid using statics unless the method is meant for other plugins to access, for example your plugin has an API. Use a constructor instead of statics.

This is more personal preference but also conventions, don’t name your main class ‘MainClass’ Name it the name of your plugin with appropriate capitalisation. For example if your plugin is called ‘SimpleMessage’ your main class will be called ‘SimpleMessage’.
 
Harry

Harry

Rustacean
Management
Feedback score
10
Posts
1,606
Reactions
876
Resources
0
TumblrFeminist said:
Also just as a good practice, these lines can be simplified.
Code: 
        if(hasPMEnabled(sender) == false) {
            sender.sendMessage(ChatColor.RED + "You currently have your private messages disabled.");
        }
        else if(hasPMEnabled(receiver) == false) {
            sender.sendMessage(ChatColor.RED + "That user currently has their private messages disabled.");
        }
Code: 
        if(!hasPMEnabled(sender) ) {
            sender.sendMessage(ChatColor.RED + "You currently have your private messages disabled.");
        }
        else if(!hasPMEnabled(receiver)) {
            sender.sendMessage(ChatColor.RED + "That user currently has their private messages disabled.");
        }
Click to expand...
HippoPlayz said:
I would avoid using statics unless the method is meant for other plugins to access, for example your plugin has an API. Use a constructor instead of statics.

This is more personal preference but also conventions, don’t name your main class ‘MainClass’ Name it the name of your plugin with appropriate capitalisation. For example if your plugin is called ‘SimpleMessage’ your main class will be called ‘SimpleMessage’.
Click to expand...
Thanks for the feedback and help guys! :tup:
 
Turtle

Turtle

turtle#1989
Supreme
Feedback score
17
Posts
751
Reactions
419
Resources
0
HippoPlayz said:
I would avoid using statics unless the method is meant for other plugins to access, for example your plugin has an API. Use a constructor instead of statics.

This is more personal preference but also conventions, don’t name your main class ‘MainClass’ Name it the name of your plugin with appropriate capitalisation. For example if your plugin is called ‘SimpleMessage’ your main class will be called ‘SimpleMessage’.
Click to expand...
You can also use statics for utilities.
 
Selim_042

Selim_042

Feedback score
1
Posts
33
Reactions
3
Resources
0
I recommend using the ChatColor as provided by Bukkit rather than the one for Bungee. The Bukkit one's package is less likely to change. There is also no need to print statements when the plugin is enabled and disabled as Spigot does that for you.
 
Turtle

Turtle

turtle#1989
Supreme
Feedback score
17
Posts
751
Reactions
419
Resources
0
Selim_042 said:
I recommend using the ChatColor as provided by Bukkit rather than the one for Bungee. The Bukkit one's package is less likely to change. There is also no need to print statements when the plugin is enabled and disabled as Spigot does that for you.
Click to expand...
nah, it's unlikely the bungee one will change either - but i'm fairly sure the bungee version is only on spigot and not on bukkit
 
Selim_042

Selim_042

Feedback score
1
Posts
33
Reactions
3
Resources
0
Turtl said:
nah, it's unlikely the bungee one will change either - but i'm fairly sure the bungee version is only on spigot and not on bukkit
Click to expand...
Still, better safe than sorry.
 
Max604

Max604

Feedback score
7
Posts
86
Reactions
24
Resources
0
Selim_042 said:
Still, better safe than sorry.
Click to expand...
It just makes more sense to use the ChatColor class from Bukkit because you're not dealing with Bungee. It's ultimately a personal preference, only a minor recommendation.
 
Status
This thread has been locked.
Top