Which is the "better" practice

Newbie & Debugging Questions
#1

Hi guys,
when I was in school, I learnt that singletons are generally bad practice. Now I got some manager classes that handle my game states, input (game actions) and game related objects like group of enemies, items and animations.
I want to access them in every class (e.g. in a menu state where I need a localized string) but I don’t want to parse my config in each state individually.
My first attempt was (obviously, I guess) a singleton. But the code ends up messy.
My second attempt was a static instance of my configuration in my game class. Is this a “better” practice? My code doesn’t look as messy as with singletons.


Game.colors.getIntegerProperty("font_highlight")

vs


ConfigHandler.getInstance().getConfig("colors").getIntegerProperty("font_highlight")

Is there even a better attempt?

Thanks in advance!

#2

I believe the second is the most-correct, but it all depends on what you’re doing. For example, I have a master “Game” class that defines a few universal constant variables that I want access from anywhere:

	private static final String gameVersion = "InDev 12 Unstable 1";
	private static final String settingsVersion = "1";
	private static final String gameName = "Retro-Pixel Castles "+gameVersion;
	private static final int splashState = 0;
	private static final int mainMenuState = 1;
	private static final int mapEditorState = 2;
	private static final int playState = 3;
	private static final String icons = "res/icon.png";
	private static Random random = new Random();
	private static int startWidth = 1280;
	private static int startHeight = 720;

These are all absolutes that will never change, and have no purpose to ever exist as anything else. So I access them via getters and settings in the main Game class;


	public static int getMinimumWidth() {return startWidth;}
	public static int getMinimumHeight() {return startHeight;}

	public static String getVersion() {return gameVersion;}
	public static String getSettingsVersion() {return settingsVersion;}

Technically, I believe this is coding taboo. But it makes sense in game design to do this, because these variables are always the same, on all machines or all specs and never change. So why pass around or get an instance of Game when they should just be a static value everything can fetch when needed?

I do something similar with my SettingsParser, it’s a mostly static class. Only one instance of the settings would ever exist, there’s no logical reason to have more than one instance, but you can change the values in game (via the settings menu). But again, there’s no logical reason why you’d have two instances of a class file that outputs hotkeys, so it’s static, and I access keystrokes with something like this:

if (input.isKeyPressed(SettingsParser.getKeyMiniMap()))
      openMiniMap();

Both of my ways may (and I’m pretty sure are) technically wrong. But one thing i’ve learned in game dev; the normal rules you learn in CompSci sometimes just don’t make sense. Game dev is a different animal, and it’s really hard (and almost needlessly complicated) in some areas to follow the rules letter-to-letter. But you should always try.

Now, adding on to this though, this isn’t a greenlight to say f**k it and just make everything static, these two classes I used as an example are the only two classes in my entire game that even have statics. When you do stuff like in my example, you need to be absolutely positive without a doubt that these things really will make sense to remain this way in the future.

EDIT: Half-awake typo fixes

#3

This is the real problem. So, first take a step back and decide what stuff is really needed at which place. Always try to limit dependencies.

#4

all em varibles are so messy

	private static final String 				gameVersion = "InDev 12 Unstable 1";
    private static final String 				settingsVersion = "1";
	private static final String 				gameName = "Retro-Pixel Castles "+gameVersion;
	private static final int 					splashState = 0;
	private static final int				    mainMenuState = 1;
	private static final int 					mapEditorState = 2;
	private static final int 					playState = 3;
	private static final String 				icons = "res/icon.png";
	private static Random 						random = new Random();
	private static int 							startWidth = 1280;
	private static int 							startHeight = 720;

better =D

#5

Other than the fact that’s not the formatting standards. :wink:

#6

The correct answer is the easiest for you to read, write and maintain. Everything else is someone else’s wishful thinking.

#7

My feeling is if the variables are static final immutables, there is no harm in making them globally accessible.

Regarding virtueel’s code formatting suggestion, I find rayvolution’s original easier to read.

#8

I don’t disagree, but the crux here is the “maintain” part. These static approaches are NOT easy to maintain.

What happens when your design changes and you need to have multiple instances of these classes around? And I know the reaction is “but I know I’ll never need that”, but no, you don’t.

This is a misuse of the static keyword, in an attempt at making it “easier” to get at the members of a class. This is going to come back to bite you eventually.

Use OOP the way it was intended, and just pass around an instance when you need it. It might feel like “more work” in the short term, but it will definitely save you (or anybody else who has to deal with your code) a ton of time and headaches in the future.

#9

^ this. :slight_smile:

That’s why I stated in my examples they’re technically wrong. The only reason I went with it is Game is really truly things that should never-ever change once the game is launched, and settings actually just actives whatever profile you selected, loads it’s settings and uses it until the game session is over. Both concepts are anti-OOP, but it’s one of the few instances I don’t mind bending the rules.

But one thing I didn’t mention was my SettingsParser is actually coded in such a way I could easily make it a non-static class, and load an instance of it when you select a profile. I did that on purpose just in case my design fails. Something I think anyone who does a coding taboo like this should keep in mind. :slight_smile:

I guess it all boils down to “don’t do stupid crap because it’s easier”, you quickly fall into a pit of god classes and weird static variables.

#10

1990’s thinking. In more years than I care to admit to…it’s never happened to me (that correcting a bad assumption consumed a large amount of time). However I see to opposite quite alot. And this is complete ignoring, as Cas quaintly puts it, the re-factoring fairy. We’re not talking about programming spreadsheets, web-servers, etc.

#11

Exactly… Whatever you do, it should be trivial to refactor, so who cares?

#12

Another problem I have with this misuse of static is that it sets a bad precedent and encourages pretty bad designs overall.

Sure, it’s trivial to refactor a little hobby game. It’s also easier to make assumptions about your context.

But what happens when you get into bigger projects and bring this style of coding with you? Next time you need access to a class member and say “well, I’ll just make it static, it doesn’t look like I need multiple instances anyway”, then fast forward 5 years just to find out that WHOOPS, you do need multiple instances after all. Now the refactoring is no longer trivial, and welcome to headache-land.

#13

I’m not a great user of static myself, particularly at work, but assuming you have access to and control of all the impacted source code, the the refactoring will be trivial regardless of the project scale. On the other hand, if you have written a library, then you are committed to your public interface for all eternity :slight_smile:

Obviously if you have written hundreds of thousands of lines of bad code then refactoring is going to be more difficult - it just means you should have refactored a lot sooner.

#14

I respectfully disagree with this.

I work with a lot of code that was written by people with assumptions similar to those in this thread. My job has been, on multiple occasions, to take an application that was originally written as a singleton and modify it to work as a non-singleton.

These applications have been written by many people over many years, all using code that misuses static references under the assumption that they’ll never need to be non-static. Refactoring them is not trivial.

Step one to avoiding these kinds of situations is to encourage people like the OP to get into good habits from the outset.

#15

“Good habits” for our projects means “use singletons all over the place”.

YMMV.

Cas :slight_smile:

#16

Get thee behind me Zoth-Ommog!

NOTE: If you summon these types of evil creatures in your code, forget not: Summoning evil creatures in contained ways via counting

Exactly…So rewind to my original statement.

Actually, let’s rewind all the way to the original question. There’s a set of data that’s read from a config file. Single, ergo static. Getting wild and crazy we can support loading different (gasp!) config files, but only one is current at any timeslice.

The details of a reasonable implementation depend on what exactly you to want support in the configuration file.

Good habits like: Never try to solve potential future problems which cannot exist.

#17

Re: KevinWorkman had a great comment earlier.

I too have had to refactor plenty of nasty code in my time for clients where general bad practices just piled up with many hands touching things in the process.

Sure… As Cas and others mention if you are doing traditional OOP and your scope is limited and you plan to not necessarily carry over ones work to the next game then do whatever works for you and be done with it.

The big problem with the singleton is versioning and modularity. I won’t dwell on things too long, but component architectures solve the singleton issue by making versioning and modularity top level concerns as part of a framework providing the tools / API to do it better. As long as one has reference to the relevant container then it’s a simple query to find the “GameData” object (or whatever used to be the singleton), etc. In my efforts there is a way to inject the relevant main container into child components without dependency injection or reflection, so that is the way I handle things.

The defining thing is are you taking a short term “library” approach or working with a framework that solves the stickier parts of traditional OOP design.

#18

I’m a big fan of the idea that premature optimization is the root of all evil. But this isn’t like implementing your own custom parallel sorting algorithm to sort a collection of 10 Strings. This is a pretty low-cost-now solution that will save you a ton of headaches in the future.

Different strokes for different folks, and different problems require different solutions, but the OP asked if there was a better practice than misusing statics to give lazy access to class members. There is a better practice: don’t misuse the static keyword to give lazy access to class members.

#19

Another potential option would be to use an enum that extends an interface. This gives you the convenience of a global static field while at the same time allowing you to add multiple instances if the need arises.


public interface ConfigProperties {
   
   void setIntegerProperty(String propertyName);
   void setStringProperty(String propertyName);
   int getIntegerProperty(String propertyName);
   String getStringProperty(String propertyName);
}


public enum GameConfig implements ConfigProperties {

   COLORS,  FONTS;

   private Map<String,Integer> intProperties = new HashMap<String,Integer>();

   private Map<String,String> stringProperties = new HashMap<String,String>();

   // Add an int property
   public addIntValue(String propertyName, int value) {
      intProperties.put(propertyName,value);
   }

   // Add a string property
   public addStringValue(String propertyName,String value) {
      stringProperties.put(propertyName,value);
   }
  
   // Get a string property
   public String getStringProperty(String propertyName) {
      return stringProperties.get(propertyName);
   }

   // Get integer property
   public int getIntProperty(String propertyName) {
      return intProperties.get(propertyName);
   }
}

Then you can access properties like GameConfig.COLORS.getIntProperty("font_highlight") from any class. If you need new categories of parameters it can be as simple as adding a new value to the enum.