Hey! I've some C++ experience (less than one year, still learning a lot) but just began studying gamedev; I've just finished SFML Game Development book book as my first contact with the subject. I'm trying to write simple 2D games on my own (Tetris, Arkanoid, long list of 2D clones with increasing complexity here etc) following the book approach, but I'm wondering if there is an alternative implementation, because the book's code very different from what I've learned in other programming books (e.g. Martin's Clean Code). 
For example, there's large parameters lists (which Uncle Bob advises against); sometimes the parameters are not really used by the class, they're just passed-by-reference/pointer to another class, which pass it to another class, which pass it... and so on, until reaching some class that really uses it. At some point the book creates a `struct Context`, which is a simple wrapper around all those many parameters. Example:

Application(): initializer list with 11 members, which calls State(Context) 	// Application stores a FontHolder and a lot of other stuff; let's see which class uses the font
        Context(): initializer list with 8 members, which is used by 		// Context have a pointer to FontHolder
        // suppose that we are in the GameState
            GameState(): initializer list with 3 members, which calls 		// GameState have a Context member, which passes FontHolder to World
                World(): initializer list with 18 members, which calls 		// World passes FontHolder to Aircraft()
                    Aircraft(): initializer list with 20 members, which calls	// Aircraft passes FontHolder to TextNode
                        TextNode(): initializer list list with 2 members, which uses the font

IMHO this long constructor lists are aesthetically ugly and smells like a big heavy monolithic class. However, being myself a beginner, I'm not sure if there's a stardard alternative. 

On one hand, I feel like it would be logical to instantiate a single TextureHolder on Application, so that we don't have one TextureHolder to load menu buttons on MenuState, another TextureHolder to load aircraft textures on GameState (I think that we would be wasting memory with so many std::maps* and speed with constant calls to load) etc; on the other hand, instantiate a TextureHolder on Application impose the burden of passing it through many (already long) initializer lists until finding some class that really uses it.

* I didn't understand why use a std::map instead of std::unordered_map, but whatever.

In FontHolder example, I would like to have something like:

class FontUser
    {
        sf::Text text;
        text.setFont(fontHolder.get(fontId)); // how can fontHolder be accessible here without passing through all that constructors above?
        text.setString("someText");
    }

My only idea was to create a singleton, but I've read here that they're bad and one alternative is "to simply pass the object you need as an argument to the functions that need it", but perhaps the author of this site didn't have a 20 argument list in mind.

I've read this, which ressonate what I'm feeling, but I don't know how to redesign. 

Quote

That said, chances are, if you're dealing with an argument list that long in your constructor, that your object is too big and does too much, as is. 

Quote

Again, this is a personal-preference thing, and there are exceptions far and wide, but if you're passing 20 things into an object, chances are good that you could find a way to make that object do less, by making smaller objects.

TL;DR: Beginner in GameDev struggling in redesigning giant class with long parameter lists and many dependencies. Reading a gamedev book that is confusing me and  (I think so, at least) conflicting with other programming books that I've read. For example, I don't know how to pass a ResourceHolder to classes that use it without singletons or huge parameter lists. 

Thanks!

P.S.: Sorry for any grammar mistakes, english is not my native language x_x

Generally lots of layers is the best design solution here, plus careful engineering that gives a good mix of performance and usability.

Think in terms of functionality and what systems own various functionality. Each system should know about the things it contains and the things it uses. Systems might need to know about some of their peers. Beyond that, systems generally shouldn't interact.  

Consider how UI forms are put together for an example.  There is an object called a button, an object called a label, an object called a list box, an object called a dropdown box, and so on. These objects can exist independent of the rest.  Then you can make a panel as a container, and give it some buttons and some dropdowns. You can write some logic to the panel that manipulates the buttons and dropdowns, and exposes an interface for text to go in the various boxes.  Then you can place the panel inside a bigger dialog box, along with a large list box, and give it an interface for it's functionality, in this case parameters for a file name, a directory, and the option between load versus save. Put that together and you've got the standard Load/Save dialog box.  Note how when you want to load or save a file you don't need to manipulate all the little text boxes and buttons of the dialog, you can follow the interface of giving it a file name, a path, and a few simple flags.

In your case, there is no reason that the world needs to know that an aircraft requires two fonts. Each object should only care about it's directly contained elements.

As for passing objects, sometimes it is appropriate to pass around a pointer to a big ball of stuff. Sometimes it isn't. It all depends on the details of the situation. Generally if any object is moving past a single level of indirection it is a smell.  Moving past two levels of indirection means it is probably getting too intimate with other objects, knowing details it shouldn't. When you've got code that relies on four or five layers of indirection in one place, you know you've got code to clean up.

Designing these systems so they are loosely coupled is difficult work. When you encounter a new situation you need experience to know what should be exposed and what should be contained. If you don't have experience, do some research and make an educated guess. If you guess wrong then you'll gain experience and make a better choice next time.

 If your code needs the parameters you have to pass them somehow.  As you stated, in your book they create a context struct, and that's how I often handle it. Alternatively you can have a class which represents the task you are tying to perform and put your "parameters" in that and then simply pass in the object(s) they work on to it's members. For instance in my code I have to sometimes tessellate a prism.  It's quite complex given the number of cases which is exacerbated by fact that I try to handle cases separately to avoid an lot of ifs in the code.  Instead of including a tessellation member function in my prism class, I opted to create a separate class for prism tessellation that contains all the relevant parameters, then the prism itself becomes the one parameter I pass around. This kind of thing is really a judgment call.

Also keep in mind there can also be performance issues with passing a lot of parameters. In general everything after the first few have to be pushed on the the stack depending on the architecture of the machine. I believe for the standard Windows x64 calling convention only the first four parameters are passed in by registers and the rest are on the stack (but don't quote me on that). So it sometimes makes sense to keep the parameter number down if you are looking to optimize performance.  This is not really an issue for inlined functions, it's mainly for stuff in the cpp files that has a lot of parameters in it.

Finally I try to avoid singletons if I can, mainly because you may want to have two of something later. It's not a hard and fast rule but in general if I can avoid it, I do. There is also an "if" associated grabbing a singleton in most cases, so that's something else to consider if you are trying to optimize things but it's probably not a big deal. 

I agree with @Gnollrunner that singletons should be avoid if possible. And In most case, you don't really need a singleton. You could just use one instance of the class/struct. Quite often there are classes that are designed as a singleton and later use cases that using multiple instance of that class make more sense appear.

Regarding the long initializer-list/constructor parameter lists. If it requires to have all of those parameters being valid at the time you create an instance, you can use Builder pattern. For example.

class Application {
public:
	Application(Context*, AudioManager*, GraphicsRenderer*, UIRenderer*, InputManager*, 
                NetworkManager*, ResourceManager*, SpriteUnpacker*, MemoryManager*, MyBossExecellencyState*);
};

class ApplicationBuilder {
public:
  ApplicationBuilder& SetContext(Context*);
  ApplicationBuilder& SetAudioManager(AudioManager*);
  ApplicationBuilder& SetGraphicsRenderer(GraphicsRenderer*);
  /* the list of set function goes on */
  
  Application* Build() { return new Application(/* super long parameter list goes here */);}
};

int main(){
  
  ApplicationBuilder builder;
  builder.SetContext(new Context);
  builder.SetAudioManager(GetAudioManager);
  /*....*/
  
  Application* application = builder.Build();
}

The code will be longer than it has too be and there'd be performance impact, but at least it is more readable. This is not to completely avoid the long parameter list, but it is used to limit the place where it's used to one place - inside the Build() function.

Also using 2 stages initialization can work fine as well. This technique is very similar to builder method. 

class Application {
public:
	Application();
  	Application& SetContext(Context*);
  	Application& SetAudioManager(AudioManager*);
  	Application& SetGraphicsRenderer(GraphicsRenderer*);

	void Intitalize();
};


int main(){
  
  Application application;
  application.SetContext(new Context);
  application.SetAudioManager(GetAudioManager);
  /*....*/
  
  application.Initialize();
}

However these are more like a bandage to a bad design decision. 

1) Mixers, Nested, Composition/Aggregation/Inheritance

2) Static objects

6 hours ago, frob said:

In your case, there is no reason that the world needs to know that an aircraft requires two fonts. Each object should only care about it's directly contained elements.

The UI design that you described is appealing to me, but I don't know how to implement it without passing through the references nightmare. For example, we create a Dialog, which contains and initialize a Panel, which contains and initialize a Button, which needs a texture from TextureLoader. How can Button get the texture from TextureLoader without the reference-passing Dialog(TextureLoader), which calls Panel(TextureLoader), which gives Button the texture from TextureLoader? Where must this TextureLoader be created??
Also, this design description sounds more logical and clearer for me. A Dialog contains and use a Panel, which contains and use a Button, which contains and use a texture, while the god-class Application from the book don't really use a font, a texture or a sound, it simple pass it through multiple constructors to another classes that will use it.
 

6 hours ago, frob said:

As for passing objects, sometimes it is appropriate to pass around a pointer to a big ball of stuff. Sometimes it isn't. It all depends on the details of the situation. Generally if any object is moving past a single level of indirection it is a smell.  Moving past two levels of indirection means it is probably getting too intimate with other objects, knowing details it shouldn't. When you've got code that relies on four or five layers of indirection in one place, you know you've got code to clean up

5 hours ago, Gnollrunner said:

As you stated, in your book they create a context struct, and that's how I often handle it.

If it's required to pass a pointer to a wrapper around those calls, wouldn't it be "better" (or "less ugly" ?) to reference the Application class itself? I mean, it contains all the ResourceLoaders anyway, so instead of passing 10+ references through constructors we could pass a single pointer to the Application instance that's running (and holding all the loaders and another zillion-of-things) and have a Application::getFont(int fontId) member function? Since Context just contains pointers to the Application's loaders, windows etc, perhaps we could use the already existing Application instance to provide all of this. Since Application controls the main game loop (processInput, update(sf::Time)...), I would say that this wouldn't increase coupling between the classes (at least not more than it's already coupled ?).
What do you think?

On 9/29/2018 at 3:15 PM, __AmandaGB said:

A Dialog contains and use a Panel, which contains and use a Button, which contains and use a texture, while the god-class Application from the book don't really use a font, a texture or a sound, it simple pass it through multiple constructors to another classes that will use it.

So break up your nested constructors?

One simple-minded way to go about it is to perform bottom-up instantiation in the main function (I am sure you can think of a smarter solution, I don't have enough details to give you much help there).

int main() {
  w = new World(); // Maybe it has more parameters than 0, but hopefully not many.

  // Make an aircraft, could be done in a separate function.
  text = new TextNode(...);
  aircraft = new Aircraft(text, ...);
  w.add(aircraft); // Add it to the world.

  ... // Other world additions

  game_stat = new GameState(w, ...); // Not sure what this is, but apparently you need it.

  // Not sure how much Context you'd still have in this setup.

  application = new Application(game_state);
  application.letsplay_the_game();
  return 0;
}

Instead of nesting everything into a deep structure in the constructors, build a small part of it, and add it to the collection, or use it in other parts.

You have the room to have many variables with all the data that you need to have around, although you may want to organize as well at some point. Eg have a TextNodeFactory instance which knows about textures, and builds TextNode instances (and other things with textures).

Have an AircraftFactory which produces Aircraft instances is also an option. Basically, you'll see patterns of the same code being copied into this main function, at which time you have to start thiking how to eliminate that code duplication (either as a separate function, or if it has much data, as a separate class).

5 hours ago, Alberth said:

One simple-minded way to go about it is to perform bottom-up instantiation in the main function (I am sure you can think of a smarter solution, I don't have enough details to give you much help there).

Quote

 

5 hours ago, Alberth said:

Instead of nesting everything into a deep structure in the constructors, build a small part of it, and add it to the collection, or use it in other parts.

Thank you a lot! Almost at the same time as you, someone on Reddit recommended me this interesting component pattern, and your and his approach seems quite close (I think so at least): create small, atomic components and glue them together to give life to high-level objects; bottom-up, as you said. Very cool! Thank you a lot! ☺️☺️☺️

@Alberth, just as a matter of feedback: worked fine. Thank you! 
I tested the idea with a Hangman game and I started coding the shoot 'em up game, the Aircraft is already on screen. Oh, and no more 20+ parameters lists ? Thanks again!
 

int main()
{
  	World world; 
  	GameObject aircraft(sf::Vector2f(50.f, 50.f));
	
  	aircraft.addComponent<SpriteComponent>(&aircraft, Sprites::Aircraft); // Sprite Component constructor takes two parameters directly,
  	//without the nested constructors and register itself in the World's Renderer class
	
  	aircraft.addComponent<TextComponent>(&aircraft, Fonts::Arial); // Same thing as Sprite Component
  	
  	...
    // other components: no more deeply nested constructors with 20+ parameters lists :)
      
    while (world.getWindow().isOpen())
    {
      processInput();
      update(sf::Time dt);
      renderer.draw();
    }

	return 0;
}
    

Thanks for the feedback, glad it works for you.

Albert