Why to use private instead of protected?

Hello,

Since I am developing a scene management architecture for jME2, I came in conflict with jme.math.Triangle:



I need to develop a derived class, but that is impossible since the variables of a Triangle are all private - so I will have to create a new class and copy-paste all the code + add a conversion step…



Since I am only using the modificators public and protected and almost never using private - The question arises: Why to use private…?



:? :? :?

So incompetent programmers don't mess up values they should not touch with their filthy "extends" fingers :smiley:

But why not just rename every private to protected in the Triangle class then?

I'm not quite sure I understand what the problem is, tho.  I mean, it's got getters & setters for all those private members.  Feel free to use those at will, but if you feel there's optimizations to be made by making those members protected instead of private . . . by all means, change them to protected, submit it for review and commit the changes to the project.  That's what open-source is all about. 



I've changed a couple pieces of JMEPhysics because I thought the concepts made more sense in the long run, or because of coding flaws based on faulty assumptions.  This stuff isn't sacred.



Just make sure you AREN'T one of those filthy "extends" monkeys.  :slight_smile:



As a side note . . . encapsulation is a good thing unless you need to optimize it away.  But . . . better to start overly-locked down, then loosen it up, especially in public APIs.



And @Huscar . . . I find the best way to deter overzealous "extend"ers is to make 'final' classes where appropriate.  :) 



"Extend please . . aw DAMN!"



-Falken

[ironydetector on]

:smiley:

[/ironydetector off]



Well, I mean, that was a question, I intended to ask this:

I know that the Triangle-class was perhaps created for high performance - wy seal those variables with "private"?

Falken224 said:

I'm not quite sure I understand what the problem is, tho.  I mean, it's got getters & setters for all those private members.

getters and setters perhaps suffer from performance loss - I don't know if javac does create an "inline" version of these primitive setters and getters - like a good C++-Compiler does, if you set the hint "inline".
So there might be difference in execution speed - a setter is a function call, while a value change of a variable is inline by nature...

Falken224 said:

Feel free to use those at will, but if you feel there's optimizations to be made by making those members protected instead of private . . . by all means, change them to protected, submit it for review and commit the changes to the project.  That's what open-source is all about.  

I've never submitted core changes - so I don't know how to do this.
And I thought that I would need to have at least a "Developer" or "Submitter" status - don't I?

Falken224 said:

Just make sure you AREN'T one of those filthy "extends" monkeys.  :)
"Extend please . . aw DAMN!"

I've never heard this expression "extend monkey". I have a background in C/C++ - so I am used to use multiple inheritance hierarchies - something I really miss, mainly because of the easy reuse of variables and "mixing / crossbreeds"...
Forgive me - I know how to use interfaces (a concept that only exists in Java / C# - and I would call myself an advanced programmer in both languages) - though I don't like this, but I know that it makes life easier for people because of the issues that sometimes arise when people are using multiple-inherance without knowing what they are doing...

Falken224 said:

As a side note . . . encapsulation is a good thing unless you need to optimize it away.  But . . . better to start overly-locked down, then loosen it up, especially in public APIs.

I am always thinking about encapsulation, but this is simply a case where it is not possible - because I want to access and change the values in the variables / attributes without using setters and getters (see above statement as explanation...)

Huscar said:

So incompetent programmers don't mess up values they should not touch with their filthy "extends" fingers :D
But why not just rename every private to protected in the Triangle class then?

Hmmm, I also question the competence of other people but since I am a polite person I am used to ask questions - that often seem stupid at first look, but need a deep knowledge about compilers and the way they convert source to byte code. So: be polite, use irony tags, if not meant seriously...

"Why not rename it?" - that was my question too - I don't know the performance difference of private compared to protected - it depends purely on the "intelligence" of the compiler and the way it operates.

Even though I am using Java, I am not one of those "fire and forget" people that do not question the performance. A good solution I have seen in old jME-code is, that sometimes static final is used to bundle and reuse variables (even though this might lead to non-thread-safe code ...). It is one way to achieve better performance - have a look at the code yourself, for instance look at OrientedBoundingBox - this class uses this mechanism - in return it is not thread-safe anymore - this is the drawback...

:|


EDIT:
I just had a look at the "getter" and "setter" of Triangle - it is a nightmare.
This code will surely lead to very low performance compared to a seperate setter/getter for each point AND a setter/getter for ALL the three points together...
It seems that class Triangle has not been created under performance considerations at all...

chuckle



That [ironydetector] thing reminds me of my XML insult conversations . . .



<insult target="yo' mama" topic="obesity" severity="extreme" punchline="TWO zip codes"/>



sigh ahh . . . the memories.



Okay, back to seriousness:



@Ogli - Sorry, man.  Not really questioning your ability or anything.  I think we all just forget the ironic/sarcastic tones don't always translate across the wire.



Here's my thing . . . all irony  / screwing around aside.  The trend I've seen is that Java developers tend to lean toward over-encapsulating, C developers tend to lean toward optimization rather than purist OO structure.  So far, my perception of the whole JME project has been that they lean a bit more toward that purist OO side of things than they do performance.  Personally, I don't think that's too bad a decision given that the 3D cards will be doing most of the heavy lifting, and a nice, clean, easy to use API that's hard for inexperienced developers to inadvertently screw up will speed up adoption faster than simply blazing fast performance.  Though I must say things seem to be running pretty well at the moment even despite all that.



And actually, I can forgive that purist bias because as you say, java can "inline" things a bit, though not quite in the way you mean.  Aside from just the compiler, that "hotspot" VM can do some fun things with optimizing code even while it's executing, I believe even up to and including inlining setter and getter craziness.  (though I could be wrong about that.)  I say for now, just get people using the tool, then later worry about squeezing every last ounce of performance out of it, especially if you have the VM helping you out a bit.



As for contributing, if you don't have committer access, you can post the changes you want to make into a new thread in the 'contribution depot'.  Assuming everybody approves, you can have a committer check it in for you, or you might even get commit access yourself.  Depends on what the mods think.



Personally, from what I've seen so far, you've got my vote.  The stuff you've been doing makes sense, and assuming the code you submit isn't totally crazy, I'm all for adding a bit of optimization.  And hell, they gave me access, so it can't be THAT hard to get.  :stuck_out_tongue:



Anyway, apologies if I came across as a bit condescending, it wasn't the intent.  Just messing around a bit and not really doing a good job of conveying the humor.



Hope you won't hold it against me.  :slight_smile:

on that note: http://img25.imageshack.us/img25/5350/w3ctwitter7998492.png

dhdd said:

on that note: http://img25.imageshack.us/img25/5350/w3ctwitter7998492.png

:D

Here you can see the differences between “Triangle.java” and my class “TestTriangle.java”:



Triangle

http://www.lostplanets.de/files/Triangle/Triangle.java



TestTriangle

http://www.lostplanets.de/files/Triangle/TestTriangle.java



I need a triangle with a possibility to calculate a plane - I want to create a convex culling polyhedron from those Triangles - you know… (for my scene managers…)



8)

Heya Ogli,



Here's the feedback.


  1. Watch your references.  pointa=pointb=pointc=Vector3f.ZERO makes them all the same object.  If you call pointa.set(1,1,1) at that point, suddenly Vector.ZERO.equals(Vector.UNIT_XYZ) is true.  Granted, your setters are also doing object assignments, so that shouldn't happen too often, but that brings me to . . .


  2. The convention I've seen in JME is that getters will return actual objects rather than copies.  Setters do NOT, however, do object assignments, but rather 'set' the values of the existing objects.  Doing assignments the way you have it will actually end up being MORE memory intensive in the long run if you're doing these assignments within a loop, since you'll be REQUIRED to create a new Vector3f object for each 'set()' call.  To illustrate:



Vector3f placeholder = new Vector3f();
for(int loop=0; loop<5; loop++)
{
  placeholder.set(loop,loop,loop); //I know it's nonsense. but bear with me.
  triangles[loop].setPoint1(placeholder);
}
//At the end, ALL triangles have a point1 of 4,4,4 . . . not what this code intended, I think.



The fix:


for(int loop=0; loop<5; loop++)
{
  placeholder=new Vector3f(loop,loop,loop);
  triangles[loop].setPoint1(placeholder);
}



now requires not only the overhead of object creation, but will also end up invoking GC calls faster as the Triangle internally abandons its original Vector3f objects for garbage collection later.

3) I'm still unclear why you can't subclass from the original Triangle class.  You haven't changed any of the underlying calculations at all, but have just added a 'isComputationOfPlanePossible()' method, as well as a 'calculatePlane()' method.

My advice is simply subclass Triangle and change the setter to the following()


public void set(int i, Vector3f point)
{
  super.set(i,point);
  isComputationOfPlanePossible = isComputationOfPlanePossible();
}



Then simply change the 'isComputationOfPlanePossible' method to use the existing getters.  Currently, it doesn't seem that this implementation really buys you much in terms of speed.  And it's a lot of code duplication for a minimal augmentation of current functionality.

Note: I realize you can't override the calculateNormal() method to return a boolean, but you CAN override it to throw a 'UnsupportedOperationException' and implement 'public boolean calculateNormalIfPossible()' method to replace it in your derived class.

Thoughts?  I could be missing a piece of the puzzle here, so take my feedback with a grain of salt.  And let me know if I've misunderstood.

-Falken

I think we should just replace private to protected for the attributes of Triangle - then I could make a fast extension of the Triangle class and add also my own setters that don't use case branches and with the last setter one can set all three points at once.



You are also right about object references - I still don't know if I should use references or clone copies in my new library.

I think I will provide a method for both.

I thought about this topic very often. I think it will end like this:

getVector() { return pointa; }

getVectorCopy() { return pointa.clone(); }

(for every class…)

Of course I will need to clean up my whole code then.

And you are absolutely right about side effects when working with those loops you illustrated…





Thanks for your feedback Falken! I will follow your advice. Could you change private to protected in Triangle class and check it into svn ?

:slight_smile:

Actually, I'd just forget about doing the 'getVectorCopy()' methods.  You should almost never use methods that create objects, and if someone want a clean copy of one of your objects, they should be pretty explicitly doing that in the code they're writing anyway.  shrug  Will save you a bunch of time writing extra 'Copy' getters.



As for making changes to Triangle class, I'd love to, but . . . Don't know if you noticed my forum rank there.  Still a Newbie, even though they're letting me commit on the JMEPhysics project.  For now, I'm pretty much banished to JMEPhysics, as there's not much activity on that one lately.  So, as much as I'd love to help you out, I'm not the one to ask about mucking with core classes.  I think for now you're stuck with using the getters, but honestly, that's an easy refactor once they change the protection level for you, and in the meantime, you should be able to at least make it all work.  :slight_smile:



Sorry to pansy out on you after telling you the hoops to jump through.  Drop a copy of this thread onto the 'Contribution Depot' area of the forum and you should be able to get a higher-level committer to help you out.



In the meantime, happy developing! :slight_smile:

Wouldn't it be easier to set those points in an array " Vector3f points " and use a simple return points; to get rid of the switch?

Just wondering since I read the new version of java will have faster/better arrays somewhere on these forums.



I didn't notice the getter… "no filthy extending fingers"-rule doesn't make a lot of sense then.



Could you perhaps calculate whatever you wish to calculate in the constructor of your extending class? Since that's were the vectors are "fed" to the triangle you don't need to access the private values but can just use those provided by the constructor call.



Oh btw, when I mentioned incompetent programmers I was not referring to anybody. So apologies if it offended you, that was never my intention.


Falken224 said:

And @Huscar . . . I find the best way to deter overzealous "extend"ers is to make 'final' classes where appropriate.  :) 

Well that's a solution if you don't want to change the values "yourself".