Class design question

[Annuate]

Hi everyone, I'm working on a summer coding project with few friends from school and we have a design issue that we need some input on.(The language is Java) We have for example a 2D vector class called Vector2. It has two properties float X, and float Y. When I was learning Java I was always told that these should be private and have getter and setter methods. The debate is now that if I want to say add up the x components of two vectors and set that to the x component of another vector we now have a bit of a mess. example

myNewVec.setX(vecA.getX() + vecB.getX());

If we had set the fields public we could have much cleaner syntax as follows:

myNewVec.x = vecA.x + vecB.x;

So aside from the nice syntax the other argument is why make the extra methods for set and get x,y if we are providing the same functionality as just setting it public anyways. Example

public float getX()
{
 return this.x;
}  

public void setX(float x)
{
 this.x = x;
}

is the same as just using myVec.x = someOtherValue; or just returning return myVec.x

So basically TL;DR Looking for the input of someone with more experience then us to help us make a decision since we do not have the getters;setters; of C# nor the operator overloading of c++. Am I overlooking something important making it public or is it justified? Is having the "syntactic sugar" worth going against what we were taught? Thanks :)

[jon.kiparsky]

I say you shouldn't do either, but if you must allow one class to monkey with another's data, you should at least keep some control over it.

Really, if one class is thinking about the fields of another class (instead of what it would like that other class to do for it) then you've screwed the pooch on your design, and you need to rm *.java and start over. That means if you see the need for a setter or a public (non-static, non-final) field, you have blown it entirely.

Why do you want to set a field of some random vector? What did it ever do to you?
Instead, ask yourself what it is you want that other class to do for you, and figure out a way to put that so it makes sense in terms of the model you're building.
The classic example is a class representing a moving object. You could have your model reach into the moving point and tweak its bits: point.x = 5; point.y = 79; That's awful of course - the object that's moving isn't modelling its own motion. What's the point, you might as well be writing assembler.

A little better is to ask the object to move itself: point.move (4, -79); Still terrible, though. Does this point not exist in a space where motion is governed? What's really happening is, you're affecting the motion of the object: point.setVelocity(4,-2) and the point itself is figuring out where it needs to be next.

Capiche? For another example, think of a Person class (always a favorite). You'll have a name field, of course, and in the 101 textbook, you'll have a setName() method. Well, why? People don't set their names, they change them. So if you have a person whose name changes, you then have some rigamarole - you have to establish that there's no reason they shouldn't do that, that the general public (creditors included) is notified, and so forth. If you just "setName()" you're not likely to think of any of that. But OO, done right, should be an attempt to represent something in the world, and in the world you rarely just "set" a value.

Public fields may seem convenient, but they always lead you into more trouble than they lead you out of. Only make a field public if you're also making it static and final.

[JamesCherrill]

Firstly, I agree with Jon (I think he's a bit too hard on field setters, but I support his reasoning behind that).
Now think about maintenance/enhancement. Writing a program is easy. Updating someone else's program when requirements change is hard.
Suppose you do your Vector2 class and develop loads of code with myVect.x style references in it. Every mathematician and physicist knows that v2d vectors can be expressed as x,y or in radial coordinates r,theta (r^2 = x^2 + y^2, tan(theta) = x/y). The two are mathematically equivalent, but the radial form is far easier for problems involving rotations, orbits, and forces that are centered at some point (eg gravity). You can add that to your Vector2 class, but you can't touch your x,y fields because all kinds of code relies on accessing them directly, so there's no way to make the two representations equally valid and performant.
If, on the other hand, you had used accessors you could add identical accessors for r and theta without breaking anything. You can play with the internal representation as much as you like (store x,y or r,theta, or both, update on the fly, update when required etc etc) and no other code will get broken.
Then next time round, when you need to deal with a normalized form as well, the same applies.
For me, the arguments for never sharing fields are pretty strong when you implement a program, but they are absolutely overwhelming when you need to update a program.

[Annuate]

First off, thank you both for responding! I agree normally if a Field was public that it should be final. If this vector was being used in real-time simulation though wouldn't recreating a new vector with a static method every time the position changes slow it down with all the object creations? I guess we could have a move function to set the positions without recreating the vector. Reason I ask is because in XNA or D3DX the x,y,z fields are public.

XNA Vector2

D3DX Vector2

I also realise that c# does have fancy getter; setters; that hide the implementation but allow for a public looking style. Point being though they still allow to get and set each individual coordinate. Which if I understand what you guys just wrote for me is not good practice and bad design choice? or is this an exception?

[JamesCherrill]

"move" method - yes (jon did make this point)

I don't see a problem with getting/setting individual coordinates, if that makes sense in the application context, PROVIDED that is done via a method call not a direct access to the field.

[jwenting]

I'd just make a method in the Vector2 class that takes 2 instances of Vector2 and creates one that's the addition of them.

So you'd get

Vector2 newVector = vec1.add(vec2);

or possibly (using static members, not what I'd do)

Vector2 newVector = Vector2.createByAddition(vec1, vec2);

Much cleaner than what you yourself tried.