Setter and Getter methods

archive documentation-jme2
1

In all my "java training" I was taught to always write "getter" and "setter" methods when writing classes. I have been working a little bit with jME and was looking through the documentation and realized there are not getter and setter methods. For instance, I had the following line of code…


Pyramid pyramid1 = new Pyramid("pyramid1", 5f, 5f);



I then wanted to make another pyramid like so...

Pyramid pyramid1 = new Pyramid("pyramid2", pyramid1.getWidth(), pyramid1.getHeight());



... or something like that. I know this isn't a practical example, but if you wanted to create these things iteratively then I am not sure.

Basically, I am wondering, why are there no getter and setter methods or is there an equivalent?
2

For most things there are setter and getters in jme.

I guess you could add getter's for the pyramids width and height. (make a post with the patch in the contribution depot)

Setting the width or height wont do anything until the vertex data etc is regenerated, which makes simple setters a bit useless i think.

3
Core-Dump said:

Setting the width or height wont do anything until the vertex data etc is regenerated, which makes simple setters a bit useless i think.


Exactly! There would be no point in having setter methods for jME's primitives, because once the constructor has finished, they will function like nothing else than a TriMesh anyway - and TriMesh has all the getters/setters you'd expect for vertex buffers etc.
Hence I don't think exposing getters for the initial properties is such a good idea either, because you can easily change the vertex buffer of your pyramid to be twice as high as it was created, so you couldn't rely on pyramid1.getHeight() returning the actual height of the pyramid anyway.
(Yes you *could* compute the bounds of the vertices in the vertex buffer but that would be a little clumsy to say the least...)
4

That is good to know. Thanks guys!

5
Core-Dump said:

I guess you could add getter's for the pyramids width and height. (make a post with the patch in the contribution depot)


But convenient for some things occasionally. Anyway, as suggested I'm writing a patch right now :) one question, r.e. documenting them, how would you define "shell samples"? As used in, e.g. disk, is this a reasonable explanation do you think or have I got it all wrong here:

    /**
     * Sets the sampling frequency used out from the centre of the disk towards it's edges.
     * <p>
     * <strong>Note:</strong> this will cause the disks geometry data to be updated, if you
     * need to alter a number of properties at once it may be more efficient to create a
     * new disk and use it to replce this one in the scene graph.
     *
     * @param shellSamples
     */
    public void setShellSamples(int shellSamples) {
        this.shellSamples = shellSamples;
        update();
    }

    /**
     * Sets the sampling frequency used radially around the circumference of the disk.
     * . . .
     * @param radialSamples
     */
    public void setRadialSamples(int radialSamples) {
        this.radialSamples = radialSamples;
        update();
    }



I've omitted a similar warning from the radial sample doc for brevity here, but it'll be in the patch.
6

Have you been able to do this for all of the shapes? Are there other classes that it would be convenient to have getter methods in?

7

I'm about 2/3 of the way through the shapes and should have the rest done by tomorrow. I've got a couple of sets of tests that I'd like to get written up as well, so with that and review time I'd expect to commit the changed towards the end of this week.



As for other changes there are a few that I'd like to suggest, but I'm hanging back until I've got a bit more experience with the framework and have used it a bit more. The shape changes are all pretty minor though, and they'll help out a lot with some of the upgrades that I'm working on for Monkey World 3D (at the moment I've got a bunch of stuff using PrivledgedAction based code to get at the field contents and it be excellent to rip it all out and replace it with straight method calls).



If there are any specific areas that you think could use some work feel free to point them out though.

8

If you really insist on adding getters for the primitive initialization data, please include in the javadocs a warning that the returned value only represents the primitive's state at it's initializaiton, not necessarily the time getHeight() is actually called.

9

I used to make a lot of getter and setter methods until I read this article Why getter and setter methods are evil?



It was very enlightening, although he mentions that in an API it is sometimes unavoidable to simplify implementation.

10

@hevee: good idea, if/when I make the change I'll make sure that I do that.



I'm not too worried about this being a problem however since some of this data is already exposed (via public fields).



In any case, if you're going to have these shapes as classes (rather than, say, methods on a ShapeFactory class that just spit out TriMeshs) then it makes sense to be able to edit them.



@MorbidMaverick: I know and I agree wholeheartedly with the author, it's a tough call though. Even his own examples don't really apply here. I'm thinking about this in terms of making these shapes easy to use in a modelling environment (sine when you're using them in a game or other app you're most likely just using them to create an initial scene graph Spatial then treating it no differently from any other graph component). So, we could make them beans and write BeanCustomizers for them, but what to do then about SWT based tools like MW3D?



I'll hang off committing anything for now until I've spent a bit more time thinking about this, there may be a better approach… (and suggestions are most welcome at this point!)