Beginner Programming Mistakes and Why You're Making Them

Discussion in 'Spigot Plugin Development' started by Choco, Oct 14, 2017.

  1. When meeting the premium resource requirements, you could always give it a shot.

    Edit: Thought you mentioned a chat plugin, not a clear chat plugin.
     
  2. Choco

    Moderator

    Again, the above still applies. I learned Java and Bukkit at the same time, but I took my time to actually properly learn Java separately from Bukkit where I thought it was required. A handful of people think that Bukkit = Java. Some couldn't write a standalone program if they were told to.

    I know you're kidding, but some people have actually attempted to publish premium resources with content like you've described.
     
  3. Choco

    Moderator

    Please remain on the topic of the thread. This is not the purpose of the thread. If you want to post memes, do it elsewhere, please. I didn't create this thread just to have people goof around in the discussion. Keep on topic
     
  4. foncused

    Moderator Patron

    Cleaned this up a bit - please remain on topic. This is incredibly useful and @2008Choco would like it to remain that way.
     
    • Funny Funny x 1
    • Friendly Friendly x 1
  5. Sorry for replying to an inactive thread. I might be wrong, but I think that the list of where you can use static should include Maps used to access instances of classes from the same class. So for example.

    Code (Java):

    public class PlayerData {

        private static final HashMap<Player, PlayerData> playerData = new HashMap<>();

        private final Player player;

        public PlayerData(Player p) {
            player = p;
            playerData.put(p, this);
        }

        public static PlayerData getPlayerData(Player p) {
            if (!playerData.containsKey(p)) {
                new PlayerData(p);
            }

            return playerData.get(p);
        }
     
        public static void removePlayer(Player p) {
            playerData.remove(player);
        }

        public Player getPlayer() {
            return player;
        }
    }
     
     
  6. A big NOPE.
    You are abusing static. A class should not handle instance of itself. Move Your static stuff into a separate class and remove the static modifier.

    That is one of the mistakes beginner make.
     
    #26 ysl3000, Jan 11, 2019
    Last edited: Jan 12, 2019
    • Funny Funny x 1
  7. I don't think it's extremely bad (like, we both know we've seen worse) but it's not best practise either.

    If you want to do it the best way possible (according to the SOLID code principles), I would recommend you to create something in the lines of a PlayerDataProvider interface and a PlayerDataProviderImpl implementation. This implementation class can have a non-static hashmap field which keeps track of relations between player and playerdata. This interface can then for example be bound and injected with dependency injection (see my signature if you're interested in that).
     
    • Agree Agree x 1
    • Useful Useful x 1
  8. MiniDigger

    Supporter

    It's completely fine if you don't want to over engineer your stuff tho. Nothing wrong with it.
     
    • Agree Agree x 6
    • Informative Informative x 1
  9. But if we don't litter our code base with heavyweight libraries, use an unhealthy amount of out-of-context jargon, and stick to the ambiguous naming conventions that lead to the bandwagon we're currently riding, how will we ever be able to get a job writing super interesting enterprise software?! Any project worth its bits must have at least a ManagerManager or a FactoryFactory and have a startup time of minimum two minutes! If we make things too simple, how do we maintain our seat on the high horse?
     
  10. MiniDigger

    Supporter

    I mean, I write Enterprise software for a living (at an insurance company, can't get more enterprisy than that) and my boss would fire me if I would write such code for wasting his money on my time and the time my coworkers spend on a code review ^^
     
    • Funny Funny x 1
  11. Thats funny, I did some code refactoring at work last seek and someone was actually making factories for factories. "ConfigFactory" and "ConfigFactoryFactory". We call ourselves an enterprise by the way.
     
  12. This is true! Well said my man.
     
  13. Maximvdw

    Benefactor

    Reminds me of the 00's where every software regardless on complexity or loading time had to have a splash screen
     
    • Funny Funny x 1
  14. What @Joiubaxas described is actually something I was taught at my university as well. The only thing I disagree with is the word "access". The reason why storing instances statically is fine is because such a storage should logically exist only once at runtime. The same goes for example for static "instance counter" fields. "Access" implies that static is misused as an access modifier for a lack of understanding of dependency injection, but the code example is OK.
     
  15. Maximvdw

    Benefactor

    Imagine, creating a child of a PlayerData class - it would completely ruin the design - introduce unneeded casting for getting player data ... You always have to keep scalability in mind
     
  16. How does a separate non-static handler fix this? Should that also be extended to create yet another handler for an extending class? TBH that sounds terribly over-engineered for a simple project.
     
  17. Maximvdw

    Benefactor

    Everything that prevents you from deleting code and writing it somewhere else is not overengineering - its planning.

    Writing it in a controlle/dao/manager or whatever you want to call it allows you to have a separation of concerns, where each "manager" manages a set of fixed models. If you want to add a "ChildPlayerData" for example , you don't necessarily need to edit the existing managers - you just add one and don't touch the code that you previously wrote (you might break it, other people might rely on it,..).

    Basically you are putting all pressure on that one class called PlayerData. If you add or edit something in that class, you have the risk of breaking things. If you want to add new things like a database,.. you have to either do it in that class or some other class - but at that stage you might as well put the map over there.
     
    • Agree Agree x 1
  18. And in the other 95% of the cases where it doesn't suddenly break in upon you that you need to extend that class for some reason, you have created what you could call a useless boilerplate class.

    I myself usually do use handler classes simply because it's cleaner in my opinion to keep both things separated; I don't want a dozen of static "getByX" methods in my class. But I disagree with calling it a question of good or bad practice, and it never came to my mind to extend one. If there are children of the managed class, I simply use generic types to avoid casting.
     
    • Friendly Friendly x 1
  19. Maximvdw

    Benefactor

    No, you have created an extension point. Something to build upon rather than edit. Compare it on a more abstract scale to Minecraft modding and Spigot plugins. If you want to edit the chat, you have to "change" the chat classes in minecraft
    If you want to edit the chat on a server, you can simply add something that hooks into it.

    The same applies to the boilerplate code. Not on a large scale like minecraft - but enough to consider when creating your own software. Software design patterns don't just exist so others can understand your code, it exists for yourself as well
     
    • Agree Agree x 1
  20.