Change visibility of attributes

[GoogleCodeExporter]

I finally had some time to browse through the code. I found many 
occurrences of not encapsulated attributes, like

    public BaseGraphics bsGraphics;

in the Game-class. That goes against pretty much everthing I have learned 
about object-oriented design.

I even saw some package-visibilities:

    GameFont fpsFont;

Plus, many attributes are private. That limits the extensibility of the 
library, as inheriting classes cannot access needed fields of the base-
class. I already ran into that problem when I tried to subclass the sprite 
class.

So here is my proposal: let's change that! I would propose to mainly allow 
two sorts of access:

protected - for the vast majority of attributes.
private - for some attributes that are only used for the inner workings of 
the class itself, with no possible need in subclasses.

And only if really needed:
package - for some rare special cases. This should be explained in detail 
in the comments.
public - only for static final attributes.

And of course, we will have to add a ton of getters/setters (all public).

I think I don't have to explain the advantages of encapsulation, so the 
upside of this should be clear. The downside, however, are some API-
changes. Instead of calling game.bsGraphics you should have to call 
game.getBsGraphics().

I am willing to do this, but first I'd like to know what you think.

Original issue reported on code.google.com by cedricbe...@gmail.com on 4 Apr 2008 at 1:40

[GoogleCodeExporter]

Original comment by cedricbe...@gmail.com on 4 Apr 2008 at 1:42

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

That is made on purpose.
The public variables only on the base engine, it's visible and clear to the
developer, what engines the Game class wrapped of. Also for easier access and 
changed.
I made it like that only for making things simple :)

Variables made protected, it's make the API docs ugly :)
All variables has it's own get/set I think.

Original comment by pau...@gmail.com on 4 Apr 2008 at 2:21

[GoogleCodeExporter]

> The public variables only on the base engine
Well I mainly looked at the classes in com.golden.gamedev - where I found some 
occurrences. I haven't really seen it in other packages, but I didn't look at 
them 
that closely.

> Also for easier access and changed.
Should these variables really be changed? The game could easily crash when one 
does 
that.

Doing MyGameObject.getParent() isn't that much more difficult then 
MyGameObject.parent, I think and it would prevent the beginners from setting it 
to 
null or something like that. Encapsulation is used in every java library, so 
everyone should know how to use a get/set-method. Perhaps it is even a little 
more 
confusing, NOT to use them.

> Variables made protected, it's make the API docs ugly :)

Hehe, that shouldn't really be an argument. Form follows function. ;-)
And as I said: making them protected would enhance the library, as everybody 
could 
create their own subclasses for specific purposes.

Original comment by cedricbe...@gmail.com on 4 Apr 2008 at 2:43

[GoogleCodeExporter]

> Should these variables really be changed? 
> The game could easily crash when one does that.

Base engine can be changed to other engine, such as changing sound engine to
JavaLayer MP3 or JOrbis OGG. If one don't know what the meaning, why they 
change it
then, and it's not easy to change it, if change it to another engine, it's still
working :) instead of setting it to null of course, and I don't think beginner 
would
dare to set engine to null :-) Also with get/set beginner can set it to null 
either,
no difference.

> Doing MyGameObject.getParent() 
> isn't that much more difficult then MyGameObject.parent

Well I only want to make it looks prettier.
The engine is used to poll the engine directly instead of what already wrapped 
in
Game class.
Such as bsInput.isKeyReleased(...) instead of getBaseInput().isKeyReleased(...)
because engine can be frequently used in game, and it makes it ugly.
Also Game class does not wrapping any BaseIO engine, so it can easily get URL by
bsIO.getURL(...) instead of getBaseIO().getURL(...), more pleasant to see I 
think.

And it's public instead of protected because of the engine can be used outside 
the
Game class, for example Sprite class can get the bsLoader to grab image.

> And as I said: making them protected would enhance the library, as everybody 
could 
create their own subclasses for specific purposes.

Everybody still can subclasses it for specific purposes, there are get/set 
methods
for the variables, so there is no difference between protected/private there.
I make it protected if I see there is big chances that the class will need to be
subclassed and the variables will be used frequently, such as making x, y 
variables
protected in Background class.

That's why I make it public, protected, private in GTGE, I know what the 
meaning of
those visibility attributes when designing GTGE :-)
Well if one said that it's not good design, we can change it, GTGE is open 
source
anyway :)

Original comment by pau...@gmail.com on 4 Apr 2008 at 3:09

[GoogleCodeExporter]

I would like to see it changed to. In my opinion its bad design even if it 
works and
maybe looks easier. Encapsulation and minimum visibility is a basic java 
philosophie
that everybody should use, trust and know of. I beginner has to learn and a 
advanced
user knows about it and apprieciates it.

On the other side i would like to warn changing api dramatically is never good,
because developers using GTGE have too change everything. So i think if we 
really
decide to change everything to conform java standard and philosophie we have to 
think
through every point and make a frozen api, because changing api within every 
release
is not good!

Original comment by jan.brac...@googlemail.com on 4 Apr 2008 at 9:54

[GoogleCodeExporter]

I agree with the basic principles, however, I think this needs to be in a 
futher 
release.  However, paupau, there is one thing with getters and setters that you 
missed...with getters and setters, you can prevent null engines where needed.

If you have a variable:
public ThisClass clazz;

and you did this:
clazz = null;

That would set it to null.

However, if you encapsulated it, it would be easy to prevent this in the setter:
public void setClazz(ThisClass theClass) {
if (theClass != null) {
this.clazz = theClass;
}
}

This is one of the reasons why getters and setters are so prevalent.  Plus, if 
you 
need to make an interface for one of your classes, you can specify getters and 
setters on the interface for properties that you require to have getters and 
setters.

Does NetBeans have a "Generate getters and setters" option?

I agree that this should be done.  However, I think this needs to be in a 
futher 
release at this time.  Remember that with the GTGE engine comes the tutorials, 
etc.  
These tutorials need to be changed varying with the changes in GTGE.  If the 
changes 
are drastic, such as encapsulating everything, the tutorials must undergo 
drastic 
changes.

Although we do support GTGE as a group, the tutorials are the first thing a new 
game 
programmer will see, and it is important to keep the established tutorials in 
mind 
as we suggest improvements for the engine.

Original comment by CarlHol...@gmail.com on 5 Apr 2008 at 1:22

[GoogleCodeExporter]

Well, I agree that the encapsulation would brake some games and that we 
shouldn't do 
that. But I still think, we should do it some time, perhaps for a major 
release, 
where some API-changes are to be expected. Let's keep this in the back of our 
heads.

Concerning the private/protected thing, I can only talk from my experience when 
I 
tried to subclass Background to create wrapping backgrounds. That didn't work 
without the source-code (which Paupau kindly gave to me at the time) because of 
some 
visibility issues. I don't exactly remember why, but I will recall it as soon 
as I 
integrate that work into the library. 

Original comment by cedricbe...@gmail.com on 5 Apr 2008 at 2:14