16

Overview

In my (Android) Java game, I have a class called resources. As the name suggests, this class holds the resources for the game. All of my OpenGL objects (Sprites) are created here

It's looks something like the following (obviously, this is a simplified version compared to that which appears in the real project):

public class Resources {

    Hero hero;
    Enemy enemy;
    MenuButtons mainMenuButtons;
    Background background;
    Scene mainMenu;

    public void createObjects(){

        hero = new Hero();
        enemy = new Enemy();
        mainMenuButtons = new MenuButtons();
        background = new Background();
        mainMenu = new Scene(this);

    }
}

So, within my mainMenu scene, I need access my objects, so we may see something like this:

public class mainMenu implements Scene {

    Resources resources;

    public mainMenu(Resources resources){

        this.resources = resources;

    }

    @Override
    public void render(){

        resources.background.draw();
        resources.hero.draw();
        resources.enemy.draw();
        mainMenuButtons.draw();           

    }

    @Override
    public void updateLogic(){

        resources.hero.move();
        resources.enemy.move();
        resources.mainMenubuttons.animate();
    }

}

Now, the above method is just one way to get access to the objects in resources and their methods. But does this actually break the Law of Demeter? If not, why not? If so, what is the best way to get access to these objects in a way that does not violate the LOD?

Accessors?

One option (which I've ruled out TBH - see below) is placing accessor methods into my resources class. So that I could do something like:

resources.drawBackround();

I have a lot of objects and I need an accessor for each method/variable of each object. Not really practical, it seems like I'm writing a ton of extra code and most importantly, it makes the resources class ridiculously long as it becomes filled with these accessors. Therefore, I'm not going down this road.

Passing in objects into the scene's constructor

Of course, I can also do something like this:

    hero = new Hero();
    enemy = new Enemy();
    mainMenuButtons = new MenuButtons();
    background = new Background();
    mainMenu = new Scene(hero, enemy, mainMenuButtons, background);

So I can simply do this:

    background.draw(); //etc....

This is workable for simple scene's (such as menu systems that don't require a lot of objects) but for the main game, it could quickly become a mess as I'd have to pass references to some 30+ objects into the constructor which doesn't really sound quite right......

So I would really appreciate if someone could point out the best way to proceed and why.

Zippy
  • 3,826
  • 5
  • 43
  • 96
  • Judging by what you've posted so far, you might want to eliminate the Resources class and use Dependency Injection instead. For Android, Dagger is a popular choice: http://square.github.io/dagger/ – Eric Levine Jun 08 '15 at 18:28
  • check the accepted answer here (http://stackoverflow.com/questions/26021140/law-of-demeter-with-data-model-objects). This may explain whether you are breaking the LoD or not. – Raghu Jun 08 '15 at 18:52
  • @elevine, I'm new to the concept of 'Dependency Injection' - I've done some reading but the level of the articles out there on the subject seem to be quite advanced. Could you provide a basic explanation/example of how I could apply this to my situation? Thanks – Zippy Jun 11 '15 at 21:55

7 Answers7

4

So I would really appreciate if someone could point out the best way to proceed and why.

The best way, in my opinion, is to keep the Resources class, make all objects private to not break the law and write accessors (but not for every object like you already ruled out).

I have a lot of objects and I need an accessor for each method/variable of each object. Not really practical, it seems like I'm writing a ton of extra code and most importantly, it makes the resources class ridiculously long as it becomes filled with these accessors. Therefore, I'm not going down this road.

I assume many objects are of the same class. So you do not have to make an accessor for every object what would really blow up the class. I a game you normally have a hero, one or more enemies and many sprites.

public class Resources {

    private Hero hero;
    private Enemy enemy;
    private MenuButtons mainMenuButtons;
    private Background background;
    private Scene mainMenu;

    public void createObjects(){

        hero = new Hero();
        enemy = new Enemy();
        mainMenuButtons = new MenuButtons();
        background = new Background();
        mainMenu = new Scene(this);

    }

    public Hero getBackground() {

        return background;
    }

    public Hero getHero() {

        return hero;
    }

    public List<Enemy> getEnemies() {

        ArrayList<Enemy> list = new ArrayList<Enemy>();
        list.add(enemy);
        list.add(next_enemy);
        return list;
    }

    public List<Sprite> getSprites() {

        ArrayList<Sprite> list = new ArrayList<Sprite>();
        list.addAll(enemy.getActiveSprites());
        return list;
    }

}

Instead of getHero() and getEnemy() you could also make a getActor() method if Hero and Enemy are derived from the same class. The getSprites() method is just an example how it could look like.

If that solution is not going to work for you, I have another suggestion.

Make the Resources class do some work.

public class ResourceManager {

    private Hero hero;
    private Enemy enemy;
    private MenuButtons mainMenuButtons;
    private Background background;
    private Scene mainMenu;

    public void createObjects(){

        hero = new Hero();
        enemy = new Enemy();
        mainMenuButtons = new MenuButtons();
        background = new Background();
        mainMenu = new Scene(this);

    }

    public void render(Scene scene) {

        this.background.draw();
        if (scene != mainMenu) {

            this.hero.draw();
            this.enemy.draw();
        }
        this.mainMenuButtons.draw();           

    }

    public void updateLogic(Scene scene){

        this.hero.move();
        this.enemy.move();
        this.mainMenubuttons.animate();
    }

}

The mainMenu then calls logic methods directly in the RescourceManager class.

public class mainMenu implements Scene {

    ResourceManager resourceManager;

    public mainMenu(ResourceManager resourceManager){

        this.resourceManager = resourceManager;
    }

    @Override
    public void render(){

        resourceManager.render(this);
    }

    @Override
    public void updateLogic(){

        resourceManager.updateLogic(this);
    }

}

I hope my suggestions helped you a bit figure out how to continue with your project.

user3141592
  • 131
  • 6
  • This is a good solution because it abstracts that logic away. If you have to add some stuff only the Resources class is updated. – prmottajr Jun 17 '15 at 21:11
  • Hi @User3141592, thanks for this answer, I like the 2nd suggestion, however I have a question. Lets say I have 5 scenes (MainMenu, MainGame, GameOver etc....), my Hero sprite has different logic depending on the scene. So when I call hero.update(); - how does it know which logic to carry out? At the moment, I have my Scene interface, and each Scene created from it overrides it's logic() and render() methods (hence the @override) - so it always knows where to call it from as the render/logic update in the scene itself is called. Hope that makes sense! – Zippy Jun 18 '15 at 15:04
  • As I can see, the ResourceManager class has already knowledge of the Scenes. So you can give the Scene as parameter to the ResourceManager.render(Scene scene) method. You could then differ the scenes with if (scene == mainMenu). – user3141592 Jun 18 '15 at 16:27
  • I edited the second suggestion to reflect that change. – user3141592 Jun 18 '15 at 16:34
  • Thanks @user3141592, however, if possible, I would like a way of doing this *without* using **if** statements - if I have many scenes to check it could get rather messy. Avoiding if's being the reason I wrote the scene Manager in the first place! (My gameloop calls render on the current scene, so something like SceneManager.getCurrentScene().Render(); - Ie, I don't have to use if's - once I set the scene, it automatically renders/updates directly from that scene. Thanks for the suggestions so far! – Zippy Jun 18 '15 at 18:04
  • What do you mean by "... each Scene created from it overrides it's logic() and render() methods"? Do you mean different Scenes share the same Resources but choose which ones to draw(), update() etc. or call different methods of the same resources? Or do they have separate Resources? – Hami Torun Jun 18 '15 at 20:28
  • So, a method for every Scene in the ResourceManager then. In the MainMenu Scene you call `resourceManager.renderMainMenu()` – user3141592 Jun 18 '15 at 23:17
  • @HamiTorun, what I mean to say is that my interface as an update() and a render() method. My various scene classes then implement the interface and overrides it's update() and render() methods.. The game loop then calls render from whatever scene is set as the current scene. – Zippy Jun 19 '15 at 00:29
  • @user3141592 OK I think I understand where you're coming from now. It's very similar to that which I have now except that instead of calling my render/logic directly from the various scenes, they then call a specific method which is located in the ResourceManager class, which can easily call the methods of the objects because it already has access to those objects? So the resource manager would have an updateLogic() and render() for each scene. Thanks. – Zippy Jun 19 '15 at 00:40
  • It makes no sense for me to write accessors... Demeter's Law is not about accesses, its about knowment: A class don't have to know how is another class internally. A slass should say to other class: "Do this!" and not "Give your internal objects so I can do this". – inigoD Jun 19 '15 at 06:50
1

You could use dependency injection and eliminate your Resources class. Then you can call functions on your fields and wouldn't be in violation of the Law of Demeter.

Here is an example using constructor injection:

public class MainMenu implements Scene {

   Background background;
   Hero hero;  
   Enemy enemy; 
   MenuButtons buttons

    public mainMenu(Background background, Hero hero,  Enemy enemy, MenuButtons buttons){

       this.background = background;
       this.hero = hero;
       this.enemy = enemy;
       this.buttons = buttons;   
    }

    @Override
    public void render(){

        this.background.draw();
        this.hero.draw();
        this.enemy.draw();
        this.mainMenuButtons.draw();           

    }

    @Override
    public void updateLogic(){

        this.hero.move();
        this.enemy.move();
        this.mainMenubuttons.animate();
    }

}

With dependency injection, you pass instances into constructors and functions instead of "newing" them inside your class. You need to manage your instances somewhere though, and there are plenty of libraries that will do that for you. Dagger is a popular one for Android: http://square.github.io/dagger/

Eric Levine
  • 13,536
  • 5
  • 49
  • 49
  • Hi @elevine, thanks, I have some questions though. This looks similar to what I'm currently doing (but with keeping my Resources class) - in that I'm passing my objects into the constructor of my Scene class. What do you mean by 'pass instances into constructors and functions instead of "newing" them inside your class" - is this not what I'm currently doing? (I'm not creating new instances withing the Scene class). Also the main point of the question is how I can deal with large numbers of objects (say 30+) without passing all of them into a constructor, how can this help me achieve that? – Zippy Jun 12 '15 at 18:23
  • By "newing" I mean something like hero = new Hero(); which you are doing in your Resources class. If you have a large number of objects, can they be grouped into a collection like a List? You can pass the list(s) to your scene and operate on all of the objects in they list. – Eric Levine Jun 12 '15 at 18:26
  • OK but I'm only creating the objects in the Resources class & then passing them to the classes that require them. The creation only happens once. So I'm unsure of the advantage of using the method you describe. Unless I'm misunderstanding, it seems like what I have now but using an external library. I'm not seeing an advantage over just passing the objects in as I'm currently doing (see 'Passing in objects into the scene's constructor' in question) Regarding lists, do you mean putting them in a list, then 'unpacking' the list in the receiving class and assigning them to instance variables? – Zippy Jun 12 '15 at 18:32
  • You were concerned about Law of Demeter, so using the Resources class violates that. By using a List, I mean you might have a List that contains a large number of objects instead of referencing them individually. If you can come up with a common interface for your objects, then you can iterate through the list and call the methods from that interface. – Eric Levine Jun 12 '15 at 18:46
  • Re the Resources class, yes LOD is a concern, but using my method in the section 'Passing in objects into the scene's constructor' gets around the Demeter problem (by bypassing the Resource class - so I can access the object's internal methods directly, just as you have done in your example). I can't see any effective difference between my method and yours in that respect? Obviously, doing this introduces the problem of how to handle many objects via the constructor, and, I will take on-board the suggestion of using a list for my many objects. – Zippy Jun 12 '15 at 18:54
1

The idea of passing a list isn't a bad first step, but it's not sufficient. Game developers have a (somewhat controversial) concept of a structure called a "scene graph" that helps them keep track of their resources (among other things). https://en.wikipedia.org/?title=Scene_graph

It's a pretty complicated concept, but you're going to need to learn about it sooner or later. There's a lot of good advice on gamedev.stackexchange.com, so I'd suggest you take a peek over there.

Here's a nice YouTube video tutorial on the subject. https://www.youtube.com/watch?v=ktz9AlMSEoA

Erick G. Hagstrom
  • 4,873
  • 1
  • 24
  • 38
  • Thanks @ErickGHangstrom, I'd never heard of a 'Scene Graph' I'll check it out, sounds interesting. – Zippy Jun 19 '15 at 01:14
1

You could create an Drawer class that handles the drawing of all the objects. Your scene objects simply need to feed the Drawer the objects that I assume are Drawable.

public class Drawer {
   public void drawObjects(Drawable... objects) {
      for(Drawable drawable : objects) {
         drawable.draw();
      }
   }
}

This is then used by Scene to draw those objects.

public class mainMenu implements Scene {
   Resources resources;
   Drawer drawer;

   ...

   public void render() {
      drawer.drawObjects(resources.background, 
                         resources.hero, 
                         resources.enemy, 
                         resources.mainMenuButtons);
   }

   ...
}

A similar strategy, using an Updater, can applied for the other methods. If your updateLogic() method makes as simple of calls as it looks, you can definitely do the same thing, by making all those objects inherit from an Updateable interface.

public interface Updateable {
   void update();
}

Hero's and Enemy's update() methods could simply call their move() methods, while MenuButtons's update() could delegate to animate(), etc.

Obviously, if you like, you can use some sort of collection instead of varargs for the parameter of Drawer's drawObjects(). I just like the nice fluency made possible by the varargs, since you don't have to create the collection.

For other tips for keeping code in line with the Law of Demeter, check out this article: Law of Demeter and How to Work With It

Jacob Zimmerman
  • 1,521
  • 11
  • 20
  • 1
    Thanks for the link @JacobZimmerman, I'll check it out, and for your suggestions. I like the idea of having a separate renderer/drawing class, the only thing is that it doesn't address the problem of having many objects. They are being passed in as method arguments here, so if I had 30+ objects, again, the method argument list would be very long but I guess I could put them in a list as others have suggested - thanks!! – Zippy Jun 19 '15 at 01:13
  • `Renderer`! That would have been a better name than `Drawer`! It can't be confused with the other meanings of drawers. – Jacob Zimmerman Jun 22 '15 at 15:08
1

Change this:

 public void render(){

        resources.background.draw();
        resources.hero.draw();
        resources.enemy.draw();
        mainMenuButtons.draw();           

    }
 @Override
    public void updateLogic(){

        resources.hero.move();
        resources.enemy.move();
        resources.mainMenubuttons.animate();
    }

With this:

public void render(){
            resources.render();
    } 

@Override
    public void updateLogic(){
        resources.update();
} 

ResourceManager don't have to know what's inside Resources. It may be one enemy or ten, it doesn't care to ResourceManager.

And so in 'Resource' you can do:

  public void update(){
        hero.update();// Cause hero may, or may not move, he makes the choice
        enemy.update();//...
        mainMenubuttons.update();//.
    }
  public void render(){
  ...
  }

More than this! you could change the "Resource" implementation with an interface and you will be programming for interfaces and not for implementations, which is cool! This way you can have a 'Resources' for in-game and another one for menus that will be used in same way: Only changing, at runtime, the concrete Resources you will be in a menu or in game!

Anyway, not always is needed to fill Demeter.

inigoD
  • 1,681
  • 14
  • 26
1

I like the concept of a ResourceManager.
But a ResourceManager should be responsilbe for loading Resources, caching and freeing them.
Rendering is definitly a Method of a Render Object.

So the Scence - render Method could delegate the rendering to it after
instantiating a Renderer and feed it with Drawables as the Renderer does not
render Resources but renderable objects.

Say:

class MainMenu implements Scene {
    Renderer sceneRenderer = new Renderer();
    AnimatedRenderer animatedRenderer = new AnimatedRenderer();
    ResourceManager resourceManager = ResourceManager.getInstance();
    List<Resource> resources;
    List<Drawable> renderedObjects;
    GameObjectController gameObjectController;


    void initializeScene() {
          resources = resourceManager.getResources();
          renderedObjects = resourcesAsRenderables();
          sceneRenderer.setDrawables(renderedObjects);
    }

    List<Drawable> resourcesAsRenderables() {
      // if resources are not directly renderable, do decoration etc
      // and return a List of Drawable
    }

    @Override
    public void render(){
         sceneRenderer.render();
    }
    @Override
    public void updateLogic(){
       moveGameObjects();
       doAnimations();

    }
    protected void moveGameObjects() {
        gameObjectController.moveAllObjects(this, resources);
    }
    protected void doAnimations() {
        animatedRenderer.render(resources);
    }


    class ResourceManager {
       private static ResourceManager instance = null;
       List<Resource> resources;
       public ResourceManager getInstance() {
          if(instance == null) {
             instance = new ResourceManager();
             instance.loadResources();
          }
          return instance;
       }
       private void loadResources() { 
           resources = new LinkedList<Resource>();
           resources.add(new Hero());
           ....
       }
       public List<Resource> getResources() {
          return resources;
       }
    }

This clearly separates the logic and responsibilities for the tasks carried out during the scene lifecycle. A resource manager which is responsible for retrieving resources and as they may take long loading times does things like caching or freeing in low memory situations hiding the details from the client. A renderer which is responsible for displaying the objects and a controller which is responsible for moving the objects. The controller itself may implement handlers for keyboard events but that is not something which must be transparent to the scene. The renderer may swap backgrounds in or out or scale or set lighting effects but the scene only calls its render method. The animated renderer is responsible for starting , rendering and stopping animations.

Peter
  • 1,769
  • 1
  • 14
  • 18
  • BTW: Instead of using singletons you could create your own application class as it is guaranteed to exist once per application and through the complete lifecycle of your application and let this class instantiate it. Singeltons turned to becoming a bad design technique. – Peter Jun 18 '15 at 21:58
0

As can be seen your Resources dont need to be recreated, instead they do use some resources that cant be reloaded (probably images).

You should share the images object within a Resource class, and create your objects within a Scene class, on the constructor of the entities you can get the shared resource that is pre-loaded.

Marcos Vasconcelos
  • 18,136
  • 30
  • 106
  • 167