2

I designed a Snakes and Ladders game. I divided into a reasonable amount of classes and I believe I did a good job regarding the OOP design.

I have one question about a decision I made for the design, though. I have a class called Config. This class contains a bunch of global variables regarding Window Resolution, amount of players, amount of snakes and ladders in game, the grid, etc. The class currently looks like this:

package com.company.app;
import java.awt.*;
public class Config {
    public static final int WINDOW_WIDTH = 900;
    public static final int WINDOW_HEIGHT = 900;
    public static final int ROWS = 10;
    public static final int COLS = 10;
    public static final int X_RESOLUTION = WINDOW_WIDTH / COLS;
    public static final int Y_RESOLUTION = WINDOW_HEIGHT / ROWS;
    public static final int NUMBER_OF_PLAYERS = 3;
    public static final int PLAYER_WIDTH = 40;
    public static final int xOffset = Config.X_RESOLUTION / 2 - 8;
    public static final int yOffset = Config.Y_RESOLUTION / 2 + 10;
    public static final int NUM_LADDERS = 5;
    public static final int NUM_SNAKES = 5;
    public static Color[] playerColors = {Color.red,Color.cyan,Color.yellow};
    }
}

I want to add a User options interface so a User can choose the difficulty (more/fewer snakes and ladders), the amount of players, etc., so I plan to change the variables to being just static instead of final so the user can choose their own settings.

Do you believe this design is bad or not scalable? How would you improve this or, better, what would you substitute this design with?

The project is uploaded on GitHub if you want to take a look; it is not finished yet, so there will be bugs.

3

A design is never inherently "good" or "bad" per se, these attributes cannot be applied sensibly without context. A good design is one which makes a program fit to its non-functional requirements in the most simple manner. And sometimes global variables are exactly this: the most simple solution which fits well into the context.

So the question you need to ask yourself is: is the design sufficient or "scalable enough" for your purpose?

  • Do you intend to run multiple threads with different player groups all within the same process? Then it should be clear you need more than one global variable for the number of players, or the difficulty. If that is your intent, then using globals is probably not scalable enough.

  • Do you have issues in writing unit tests because of these globals? Maybe because the tests depend on some of those globals, and you have different tests which require different values for those variables, and a different order of execution may cause unwanted side effects through these globals? Then using globals would not be a good fit any more.

  • Do you expect to run into maintenance issues because of those globals, since you plan to maintain and extend your program over several years? Maybe because those globals make the code and how it works hard to understand for another person? Then global variables are the wrong tool.

But if the answer to all of these questions is "no", since this is only a toy program, just for learning purposes, then it is perfectly ok to use globals for now, and refactor later when you encounter the first issues with them. Just make sure you keep your program DRY enough, so you don't have to change 500 places when you later want to decide to replace a global variable by a non-global one.

  • This particular project doesn't really have a purpose, it was a boring evening and it proved to be fun to create.I see where you are going with this thought process.You have been helpful,I will accept this as the answer :) – Yosoku Denkai May 2 at 11:09
  • @YosokuDenkai: thanks for the feedback. Let me add that overdesigning programs is not necessarily a problem of learning examples, because even professional developers sometimes tend to add a lot structure to a program "just in case", even if that structure solves a problem for which it is not clear if it will really occur through the whole life time of the program. A rule like "don't use globals" is not an end in itself, it is a means to an end and should be treated accordingly. – Doc Brown May 2 at 12:00
  • Yeah I see what you mean.For a silly game like Snakes and Ladders theres no need to overcomplicate the design "just in case I need to launch it at google store".But as a senior student I try to keep the design clean and readable for good practice,since my older projects are pretty much spaghetti and I want to improve this skill. – Yosoku Denkai May 2 at 12:08
2

In addition to @DocBrown's answer.

Q: Do you believe this design is bad or not scalable? How would you improve this or, better, what would you substitute this design with?

Locating constants in specific classes is not bad per se. Clustering all the constants in the very same class, at the very same level of abstraction could be problematic. 1

I would consider 3 things mainly:

  1. Make the class final so that nobody can extend from Config. It's terrible when every single class extends (or implements) this sort of classes just to get access to the constants. At some point, such a class becomes the One Ring - "One Ring to rule them all, One Ring to find them, One Ring to bring them all, and in the darkness bind them". Such practice is a strong code smell because it grants components (of all sort at different levels of abstractions) access to data that otherwise should be injected.

  2. In line with the levels of abstractions, consider moving these classes to the top levels within their respective boundaries. Make'em reachable only by high business policies (abstractions) and make these to inject the values to the lower levels (concrete components). Don't allow access to constants indiscriminately from any place. Keep under control the scope of these constants. The opposite makes no difference with Locator pattern.

  3. Segregate constants per scope|boundary|layer|concern whatever. ViewConfig, PersistenceConfig, GameConfig, etc.


1: Constants are (usually) default values and not all of them belong to the same layer or should be in the scope of every single module|component.

  • You have very interesting suggestions as well,loved the Ring quote :) Can you elaborate a little bit on your second point?For example the class Tile which I use to represent a square on the screen,with x,y,width height etc.This class takes information from the Config file (X/Y_RESOLUTION,Offets ).You mean that I should make a parent taking information from Config and that Tile should extend the parent?I really liked your third point and already implemented it. – Yosoku Denkai May 2 at 11:52
  • No. Not extending. If Tiles needs x, y and width, instead of hardcoding defaults or declaring defaults in Tiles, you could delegate to the creator of Tiles the access to the constants. For example TilesFactory creates Tiles. The factory allows you to create tiles with specific parameters factoy.create(x,y,width) or a default one createDefaultTile(). The factory is who knows about the Constants. Or, even better, delegate to the Factory initializer to inject the default values during the factory initialization. – Laiv May 2 at 12:19
  • Aha I see,but what's the advantages of theese designs though? I feel like it is unnecessary to take this extra step.Perhaps you can link me with some sources to expand on this approach? – Yosoku Denkai May 2 at 12:24
  • The gain is on limiting the number of components coupled to the Config class. For example, in modular applications, these sort of classes (Config) are hard to place if they are reachable by all sort of modules and components. Modularity is a good property for any application that needs to be scalable. Modularity and decoupling are usually hard to achieve but the benefits outweigh the costs. – Laiv May 2 at 12:31
  • I see,this was indeed constructive; I will be sure to expand on this approach – Yosoku Denkai May 2 at 12:40
0

Yes the design with those globals is bad because it is hard to change them or make them into variables without breaking lots of other places in your code(tight coupling). The point is that the code is not flexible to change and when working in the real world, change is common! It is software, we want to be able to change it easily.

Here is some design feedback:

You are mixing up unrelated data. Why is the UI data e.g. WINDOW_WIDTH and WINDOW_HEIGHT are in the same class as the NUMBER_OF_PLAYERS?

Is WINDOW_WIDTH in pixels? //change the name to WINDOW_WIDTH_IN_PIXELS also if so, WINDOW_WIDTH would vary between who is using it.

The UI class should be separate from the game logic class. So I could switch out UIs e.g. text based to gui based So the UI data, even if it is constant for now, should be in the UI class a private member. The game data, e.g. number of players, should be in a game class.

Let me know if you have more questions.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.