1.8.8 A few questions about beginner programming and OOP

Discussion in 'Spigot Plugin Development' started by notjacob1, Feb 21, 2020.

  1. This is mainly going to focus on static usage
    First: is this static abuse? This is some code from my ChatUtil class
    Code (Java):

    public class ChatUtil {
      public static String cl(String msg) {
        return ChatColor.translateAlternateColorCodes('&', msg);
      }
      public static void print(String msg) {
        Bukkit.getConsoleSender().sendMessage(msg);
      }
      // etc...
     
    No instances are being created of this class, it’s purely for utility and is static for ease of use (also in this case should I make the class final?)

    2nd:
    Code (Java):

    public static ArrayList<IUser> userMap = new ArrayList<>();

    public static IUser fromPlayer(Player p) {
        for (IUser user : userMap) {
            if (user.getPlayer().equals(p)) {
                return user;
            }
        }
        return null;
    }
     
    This class isn't being instantiated either but i've heard it's bad practice to make things like ArrayList's static
    3rd:
    Code (Java):

    public class AsyncHandler<V> {
        private static ExecutorService pool = Executors.newCachedThreadPool();
        private Supplier supplier;
        public AsyncHandler(Supplier<V> method) {
            supplier=method;
        }
    // etc...
     
    This class is being instantiated but I have the thread pool as static so it doesn't create a bunch of thread pools (it's bound to the class)

    Thanks
     
  2. You shouldn't be using static "for ease of use".
     
  3. drives_a_ford

    Moderator

    This is a utlity class. I would consider the use case fine (although some do argue even on that).
    The class should be final and it should have a private constructor (to avoid instantiation).

    The static keyword is not an access modifier. It denotes that something is directly related to the class rather than an instance. In this case, I don't see any reason this field or the method should be static. You should remove the static keyword and simply create an instance of the class that is used.
    If you know that there will only ever be one instance of this class, you can enforce the singleton pattern. Although this is not always considered good practice (for good reason).
    Furthermore, in you current state the field should be of type List<IUser>. You should hide the implementation, because it's not important. This is commonly called the Liskov substituion princple.
    Although given the method you've included, I'd use a Map<UUID, IUser> instead (and the HashMap implementation). That way you can simply use Map#get in the method.

    If all instances necessarily use the same pool, it makes sense to have it static. It belongs to the class, not the instance.
    Whether or not that is the case in your example is not clear from this isolated example.
     
    • Like Like x 1
    • Agree Agree x 1
  4. You also might want to respect the Liskov Substition Principle with your second sample. It is sufficient to have a Collection<IUser> users = new ArrayList<>(); - though you might really want to reconsider using a mapping here to reduce the lookup time to a minimum.