0

I have the following class with the following function (some implementation details have been skipped):

public class TitleScreen : GameScreen
{ 
    public Texture2D titleScreen;

    public override void Draw (GameTime gameTime)
    {
        ScreenManager.SpriteBatch.Draw (titleScreen, new Rectangle (0, 0, ScreenManager.Game.Window.ClientBounds.Width, ScreenManager.Game.Window.ClientBounds.Height), Color.White);
    }
}

This class is associated with a class "ScreenManager", which is associated with a Game, which is associated with a Window. As you can see, I navigate through several classes in order to obtain the bounds of the Window so I can draw it.

My question is: Is that a bad practice? Am I coupling the TitleScreen class with Game and Window? It would be better to implement a function in ScreenManager to return the Width and Height of the window?

svick
  • 10,049
David Jiménez Martínez
  • 604
  • 1
    In short, yes, you are creating a (very weak) type of coupling to Game and Window. There are different schools of thought on this topic, one saying that you should never navigate multiple objects like that; but this leads to an unscalable cascade of getter methods on all your classes (from almost everything to almost everything else, as required). You could also use IoC / DI to inject the relevant Window / Bounds into your TitleScreen. It might be worth the effort, or you may be overthinking it / prematurely optimising the architecture. – Daniel B Dec 04 '13 at 06:50
  • To add to the above, here's a related question on SO: Java getter chaining bad or good?. I tend to agree with the "Clean Code" / Bob Martin interpretation and allow this in most basic cases. – Daniel B Dec 04 '13 at 06:54
  • Thank you Daniel for your answer, it has been very clarifying and I could get knowledge of the Clean Code book. Any way I could mark your answer as correct? – David Jiménez Martínez Dec 04 '13 at 09:22
  • @DavidJiménezMartínez I'm glad it helped. I'm under a bit of time pressure at the moment, but I'll check back later and add an actual answer (if nobody does it in the meantime). – Daniel B Dec 04 '13 at 09:31
  • @DavidJiménezMartínez posted an answer, with a bit of additional info. – Daniel B Dec 04 '13 at 15:10

2 Answers2

2

To round out my comment above:

The guiding principle involed - The Law of Demeter:

The principle in question here is the Law of Demeter, which very loosely states (taken from Wikipedia):

[Objects should] only talk to [their] immediate friends.

In this sense, your code seems to be violating a fairly important guiding principle of object orientation. So one possible solution here would be to only talk to ScreenManager, and ask it to give you the ClientBounds. Unfortunately, you may not be in control of the code (i.e. ScreenManager, etc), so this may be impossible. Even if it is possible, you can see that ScreenManager cannot simply call Game.Window.ClientBounds without breaking the Law again, so it needs to ask Game to get the ClientBounds. Depending on who you ask, this gets unmaintainable real quick. Wikipedia's article goes on to give the following warning:

Although the LoD increases the adaptiveness of a software system, it may also result in having to write many wrapper methods to propagate calls to components; in some cases, this can add noticeable time and space overhead.

A different take on the principle - as DTOs:

Robert Martin's generally excellent book Clean Code calls the above chaining of calls a "train wreck", both for its looks, as well as the ability for something to go wrong. However, a distinction is made (hopefully Uncle Bob doesn't mind my quoting here, with some modifications):

Are these two snippets of code violations of the Law of Demeter? Certainly the containing module knows [snip]... That’s a lot of knowledge for one function to know. The calling function knows how to navigate through a lot of different objects. Whether this is a violation of Demeter depends on whether or not [list of objects, in your case: Game and Window] are objects or data structures. If they are objects, then their internal structure should be hidden rather than exposed, and so knowledge of their innards is a clear violation of the Law of Demeter. On the other hand, if [they] are just data structures with no behavior, then they naturally expose their internal structure, and so Demeter does not apply.

So, in summary, the Law of Demeter might not be violated, if we see the intermediate objects as primarily DTOs (objects with no behavior).

A possible suggestion: IoC

If you are getting to the point where this type of thing becoming a concern, you can always consider Inversion of Control. In short, we're talking about taking out the logic of how to obtain the ClientBounds object out of the TitleScreen. In general then, TitleScreen would expect to be provided with a ClientBounds around the time of its construction (although there are a variety of other approaches, such as property or method parameter injection); shifting the responsibility of knowing about the full object hierarchy somewhere else (typically some wiring code).

While this doesn't necessarily solve the violation of the LoD, it does give you an opportunity to remove some duplication from your project. You also make TitleScreen less dependent on the entire object chain, and potentially more unit testable.

It's completely possible to do this by hand, but to make IoC more realistic and bearable, people normally use dependency injection frameworks, such as Unity or Ninject.

Daniel B
  • 6,194
0

What your title screen needs to know is its width and height. Where this information comes from should be none of its concern (loose coupling).

When the width and height are constant, I would pass them to the constructor.

When they might change in some well-defined circumstances, I would create a setter for these variables (currentScreen.setSize(width, height)).

When they change all the time, I would pass a reference to the ClientBounds object to it so it can check it everytime before drawing a frame.

For the rationale behind this, you might want to read about the principle of dependency injection. When an object depends on an object of another class, it shouldn't get that object itself. It should receive it from the outside. This makes your code much more modulary and flexible.

Philipp
  • 23,306