0

Edit: Have edited to provide more spefic code

As you can see my sprite drawing is breaking this rule.

I would be really grateful if someone could explain by way of pseudo code based on my code below (This is because I've read many explanations of this rule but I still don't really understand why it's a problem or how to do what I need to do without breaking this rule :-( )

1) Why would this cause a problem in my code?

&

2) Please explain an alternative way of doing what I'm attempting to do (while keeping a separate resource class that is specifically for loading my resources and creating my sprite objects).


Is there anything wrong with accessing objects through 2 or more class objects. I will explain through some code:

Here I have 3 classes, is there anything wrong with accessing the method from class2 through another object as in the third class below..........:

Code

My Resource Class

//Resource class
public Class Resource(){

    Bitmap myTexture;
    Quad mySprite;

public void LoadResources(){

    //Load graphics
    myTexture = BitmapFactory.decodeResource(view.getResources(), R.drawable.myBitmap);

    //Create my objects
    mySprite = new Quad();                //Create new Quad object
    mySprite.setTexture(myTexture);  //Set texture to this quad
    mySprite.setSize(100,100);           //Set size of this quad


    }  

}

My Quad Class

public class Quad(){

//This custom class has the bulk of the code to create all of the Quads / Sprites

public void setTexture(Bitmap textre){
//etc.....
}

//etc....


}

public void draw(int x, int y){

//Draw code here

}

And finally, my main GLRenderer class:

public class MyGLRenderer implements GLSurfaceView.Renderer{

    Resource res;

    public MyGLRenderer(){

    res = new Resources();  //Create object to my resources

}

public voide onDrawFrame(){



    //Here is my main loop and I need to draw my sprites here, so........

    res.mySprite.draw(x, y);    //Draw my sprite

}
4
  • class2Object is not a member of SomeClass so the above code will not compile. Commented May 10, 2013 at 19:07
  • I think he wrote this for example @MikeQ Commented May 10, 2013 at 19:08
  • Yes, this was written purely as an example. Commented May 10, 2013 at 19:11
  • Check my edit. I think it should help you. Commented May 11, 2013 at 19:48

3 Answers 3

7

Why it's bad to have several chained method calls

The law this violates

This violates a coding practice known as the law of demeter. This law states that you should only talk to those classes "next to you".

Why this is a bad thing

The reason for this is because, by calling methods in several other classes, your class needs to know about those methods, and depends on those methods changing. This is called close coupling. If the methods change, you need to change lots of code in other classes. This is called "Shotgun Surgery" and isn't a desirable feature in a program!

A possible solution

Look into the proxy design pattern. It's primarily designed to provide an interface to another class, and it might help you here. Perhaps by having a reference to both objects, this class can provide a common interface for all methods to talk via and reduce the coupling between the classes.

Edit to help with your example

Your example isn't actually that bad. You get an object, and you make a method call on it. The dependency comes in one form: If you remove the mySprite field from your class, your code won't work, and if you remove the draw method from your sprit it won't work. To me, the easiest solution is to add a Proxy method to your Resources class, called draw() that accepts a sprite as an argument.

Secondly, perhaps instead of accessing mySprite directly, you can put it through a method. Let's say you had a member that looked like this:

private ArrayList<Quad> sprites = new ArrayList<Quad>();

This means that in order to gain access to these sprites from the outside, you would need to have some sort of method. Now, by forcing other classes to talk via these methods, you're reducing coupling. This is because the original class only needs to know the one method in another class, and the other class will do the work. Then you wrote a drawSprite method that looked something like:

public void drawSprite(int index) // Index really is up to you {
      sprites.get(x).draw(); 
}

Now I know it might have more parameters than that, but it only means one method call from your MyGLRenderer class, hence conforming to the law of demeter, and reducing coupling in your classes.

Sign up to request clarification or add additional context in comments.

4 Comments

very nice explanation sir .. +1 for you.
Thanks @ChrisCooney, so if I'm understanding correctly the 'proxy' method would sit in my 'Resources' class and all it would do is simply pass what it receives (from my 'MyGLRenderer' class) through to my 'Quad' class to draw it. And the advantage is that if I ever renamed or removed my draw() method (from my 'Quad' class), then my MyGLRenderer class (which, ultimately makes all the calls to it) wouldn't break as such, and I would only need to alter proxy method in the Resource class (proxy method) - am I correct in my understanding? Thanks again.
Again, thanks @ChrisCooney, I understand now - it's always easier to understand with an example that relates to one's own situation - cheers!
Not a problem. Be sure to ask again if you need help :)
1

Sorry, but it wouldn't work...

As Mike pointed out, Classes instantiated in any of SomeClass's methods are "alive"/valid only when you're inside the method. This means that the instantiated object is specific to the method, and can't be accessed outside the method.

A possible solution:

The solution to this is to add a SomeClass2 member to SomeClass. If you instantiate this member in SomeClass's constructor, it'll stay alive till SomeClass is in scope, making it available for you to use. The code would look something like this: SomeClass:

class SomeClass
{
    // Define the class attributes. This class can be then accessed publicly
    SomeClass2 class2

    // Constructor - Instantiate all your members here
    public SomeClass()
    {
        // Instantiate class2 in the constructor
        this.class2 = new SomeClass2();
    }

    // Your method which does what you need
    public void classMethod()
    {
        class2Object.class2Method();
    }
}

SomeClass2 will stay the same

And SomeClass3 will be:

class SomeClass3
{
    public void class3Method()
    {
        SomeClass classObject = new SomeClass();

        // Call the instantiated member and then call it's method
        classObject.class2.class2Method();
    }
}

This should compile/run. I would comment on the design, but I'll wait to find out what you're planning to use this for before I do! :)

3 Comments

Thanks for this @mogambo, very interesting. I've basically created a resource class where I load all my graphics, create all my objects and to do so, I need to access variables from 2 other classes, I have now created direct access to both classes rather than accessing them in the 'daisy chain' way I was :-)
My code has been edited to provide a more specific example of what I'm currently doing.... thanks!
To be honest, I wasn't sure of your experience :) Looks like you know what you're doing, and I probably pointed out a syntax problem. Glad to help! :D
0

It will work (except you your SomeClass2 object is declared within the method, and it should be within the class as a member variable, but I assume that's a typo).

But I would consider using getters and setters instead (it's considered better practice):

class SomeClass{
    private SomeClass2 class2Object = new someClass2();

    public SomeClass2 getClass2Object() { return class2Object; }
}

class SomeClass2{
    public void class2Method(){
        //Some code here
    }
}

SomeClass3{

    public void class3Method(){

        SomeClass classObject = new SomeClass();
        classObject.getClass2Object().class2Method();
    }
}

2 Comments

Thanks for this @tbkn, any idea why it's not considered 'good' practice? I have a lot of code using this, so if it's not going to cause any issues, I will probably leave it, but it it might cause problems, I will probably change it. I've edited my code too to resolve the declaration issue, yep, it was a type :-) Thanks!
It's not going to cause issues. It's not considered good practices because it creates strong dependency between classes, and can cause errors (user errors) when trying to modify the code. If it's already written I wouldn't change unless you have to, but in future code try to limit the dependency of classes on other classes as much as you can.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.