views:

526

answers:

4

I'm writing a equals(Object obj) function for a class. I see that it is possible to access the private fields of obj from the caller. So instead of using a getter:

Odp other = (Odp) obj;
if (! other.getCollection().contains(ftw)) {

}

I can just access the field directly:

Odp other = (Odp) obj;
if (! other.collection.contains(ftw)) {

}

Is this bad practice?

+2  A: 

I tend to always use getters, because sometimes a getter isn't just "return(foo)". Sometimes they initialize things if they're null, or have some debug logging in them, or validate the current state in some way. It's more consistent.

dj_segfault
A: 

That is fine and completely normal. It is a little bit odd to think that this can fiddle with the private fields of other, but it's okay because there's no way anything bad can happen as far as some third party being able to muck with an Odp object's internals. Any method of the Odp class can modify any private members of any Odp object, even non-this ones, but that's fine since any such methods can obviously be trusted!

John Kugelman
+3  A: 

No, it's not. The reason that private variables and methods are not accessable from other classes is to allow you to change the internals of your class without having to change all the code that uses the class (that and to prevent the user of your class from e.g. setting a variable to a value that it's never supposed to have).

If you use private variables of other objects that doesn't hurt anything, because if you'd restructure your class's internals, you'd have to change the code inside the class anyway.

sepp2k
Just because it will work 99% of the time doesn't mean it's the right thing to do. It breaks encapsulation and couples the classes more tightly for no other reason than saving typing five characters. Please see my answer.
dj_segfault
"couples the classes more tightly together" - which classes? There is only one class concerned here, and you can't "uncouple" a class from itself.
sepp2k
Not necessarily. The parameter passed to equals() is not necessarily the exact same class.
dj_segfault
^ u mean, ClassCastException? ;)
Zefi
+1  A: 

I dont think this is bad practice, but a feature of the language. It not only allows you to test equals the way you do, but it is also useful in a Prototype pattern for object creation.

akf