How to approach my setters

[Dave Elwood]

I've got a very complicated toy project going dealing with ellipses, vectors, Point2D objects and graphics.

Some of the objects I have are very complicated, the members of each class are intertwined with other members.

Example. My vector object : if I change the length, then I must change the endpoint. If I change the direction I must change the endpoint too. If I change the length then I must change the endpoint and if I drag the source of the vector I must change the magnitude, endpoint and sourcepoint. If I drag the endpoint elsewhere then magnitude and direction also change, etc. etc.

Now : is it good programming to do this or this ??

// strict adherence to the name of the function, ONLY change magnitude
public void SetMagnitude(double mag) {
     magnitude = mag; // make adjustments to the other members from the calling object
}

//      OR

// also change values which are affected by this change of magnitude
public void SetMagnitude(double mag) {
     magnitude = mag;
/*     and change the value of the end point
     and change other things as well */
}

sort of a philosophical question.

[Rob Spoor]

Your Vector class has some invariants. One of them is that the distance between the two points equals the distance. That means that if you modify the end point you must modify the distance as well, and vice versa. You definitely do not want to let calling code ensure this. If that calling code modifies only the end point your invariant is no longer valid. The only way to maintain the invariant is to do the updates together, in your own method. Don't rely on calling code to do things you need done in your own class.

[Matthew Brown]

Rob's right. The question I'd suggest asking is "which class should be responsible for making sure the vector stays internally consistent". The only sensible answer is "the vector class" - so it's best to make it impossible for the calling class to leave a vector in an inconsistent state.

[Stephan van Hulst]

Also, consider making your vector class immutable. It's a prime candidate.

[Bear Bibeault]

And start method names with a lowercase character.

[Dave Elwood]

So the second example does it.

>>Don't rely on calling code to do things you need done in your own class. <<

Thanks for all the support. I was changing my mind on this question about twice a week and spent a lot of time modifying code.

Now all I need is to decipher two new unknowns :

invariant & immutable.

[Winston Gutkowski]

Dave Elwood wrote:Now all I need is to decipher two new unknowns:
invariant & immutable.

If you're asking what they mean, the answer is: roughly the same thing, except that the context is usually different.

An 'invariant' is usually some component of a problem or a function that never changes: for example to calculate the area of a circle, the 'r' would be a variable and 'π' would be an invariant. Invariants are often constants, although the 'squaring' function in the previous example might also be referred to as an invariant.

"Immutable" means "cannot be changed", and (at least in the Java world) refers to a way of writing a class such that it's contents cannot be changed. The best-known example of an immutable class is String, although the wrapper classes (Integer, Long, etc.) are also immutable.

Winston

[Matthew Brown]

So if you were to make your vector class immutable, that would mean:

- You set the state in the constructor, and don't have any setters, only getters.

- Any operations on the vector actually return a new object, rather than changing the old one.

[Rob Spoor]

Winston Gutkowski wrote:An 'invariant' is usually some component of a problem or a function that never changes: for example to calculate the area of a circle, the 'r' would be a variable and 'π' would be an invariant. Invariants are often constants, although the 'squaring' function in the previous example might also be referred to as an invariant.

Actually, an invariant is a constant expression. It can be a constant, but more often it's a relationship between two or more variables. For instance, the following simple classic example:

int[] array = ...
int sum = 0;
int i = 0;
// invariant: sum is the sum of all array elements before i
while (i < array.length)
{
    // the invariant is valid
    sum += array[i]; // invariant temporarily broken but that's allowed within loops or methods
    i++; // and it's valid again
}

[Dave Elwood]

To the gentleman who suggested doing away with all setters : I'm trying to save time here.

As I understand it doing this :

Point2D pt = new Point2D.Double(1.0, 2.0);

takes much more time than doing this :

pt.setLocation(1.0, 2.0);

I would save an enormous amount of work if I solved each change of color, direction, position and magnitude for my vectors with a call to "new".

However then wouldn't my animation become a bit sluggish??

[Aditya Jha]

Dave Elwood wrote:To the gentleman who suggested doing away with all setters : I'm trying to save time here.

As I understand it doing this :

Point2D pt = new Point2D.Double(1.0, 2.0);

takes much more time than doing this :

pt.setLocation(1.0, 2.0);

I would save an enormous amount of work if I solved each change of color, direction, position and magnitude for my vectors with a call to "new".

However then wouldn't my animation become a bit sluggish??

I think you're getting it a bit wrong. The effect of an immutable Vector would be:

Vector v1 = new Vector(...);

Direction newDirection = ...

/*
 * Suppose we want to change the direction of v1.
 * Instead of just:
 * 
 *     v1.setDirection(newDirection);
 * 
 * which would have modified the state of 'v1', you would 
 * work with an immutable Vector like this:
 */
Vector v2 = v1.redirect(newDirection);

As for sluggishness, don't worry too much about new objects getting generated. You can perform a memory profiling, if you really want to see how effective Garbage Collection is proving to be in your case. Usually, we follow the rule that we don't worry about it unless there is some evidence that too many objects are creating a problem. More often than not, memory issues are due to long life of heavier objects, and not from frequently created (and garbage-collected), short-lived objects.

[Stephan van Hulst]

You can still use methods like setLocation(), or whatever you want to call it. You just need to make sure they return a different object, instead of altering the state of the current object. Personally I like to start these methods with "with" instead of "set", because it makes it more obvious that they don't alter the original object. And by the way, vectors don't have colors. If you want to color your vectors in an animation, you should store the color somewhere else, possibly in a Map<Vector2D, Color>.

[Dave Elwood]

To Stephan and Aditya :

>>You just need to make sure they return a different object, instead of altering the state of the current object.

to which I say : To what end?

I've seen explanations of "wow, then your class is immutable" or imperviant or insurgent or something. If I make my class invariant or intractable what does this get me?

(And I'm working with my own Vector class called "MyVector" and it contains a graphic object from a class "VectorArrow" which does indeed have a color and a nice little point at the end (two objects of the Blade class) which remains correctly oriented to the shaft.)

There's lots of dragging and dropping and stretching of this MyVector object and with numerous calls to a MyMouseMotionListener, I fear that a new operator will slow down the visual effect.

However if you say it has little effect then I'll go the easy way and use the new operator with gay abandon.

Still though, I wonder what the sense of member variables in a class is when you garbage collect each new object left and right without any concerns for consistency.

Seems like the rich guy who turns in his expensive car when the ashtray is full. Know what I mean?

[Winston Gutkowski]

Dave Elwood wrote:There's lots of dragging and dropping and stretching of this MyVector object and with numerous calls to a MyMouseMotionListener, I fear that a new operator will slow down the visual effect.

However if you say it has little effect then I'll go the easy way and use the new operator with gay abandon.

Still though, I wonder what the sense of member variables in a class is when you garbage collect each new object left and right without any concerns for consistency.

Seems like the rich guy who turns in his expensive car when the ashtray is full. Know what I mean?

Programming is always about tradeoffs, so my suggestion is: rather than speculating about it, why not test it for yourself?

What I can tell you is that a call to 'new Object()' (which probably represents something close to the basic overhead for creating a new object) takes less than 10 nanoseconds on my old clunker Dell (averaged over 10,000,000 calls), or roughly the same length of time as:
1. A for loop that counts to 8.
2. One third of a call to Random.nextInt() (25ns).
3. One fifth of a call to System.currentTimeMillis() (45ns).
4. One hundredth of a call to System.nanoTime() (1µs).
Now that figure is in uncontested code and may be a lot less than what it takes to create your Vector, but I think you can still take it that object creation is pretty quick.

Indeed, I think I'd be more worried about 'memory bloat' than I would be about execution time.

Winston

[Stephan van Hulst]

Immutability gets you that your class is very easy to use. Especially in multi-threaded environments. You may use your vector in a graphical application now, but it's sufficiently general that you may want to use it somewhere else later. If you wrote the class with the extremely pleasant quality of being immutable, you can save yourself a lot of headache.

Why are you worrying about performance before you have even seen how fast it's going to be? I can't imagine it's going to be slow, especially since you can only drag one arrow at a time.

And you really shouldn't give your vector a color, or any other properties that determine how it appears visually. This is *not* the responsibility of a vector. Instead, maybe you can make an Arrow class in your view package, which encapsulates a vector, and adds methods that draw the arrow.

Anyway, here's how I would make the vector class. This example assumes that Point2D is immutable as well.

/** 
 * Example of a two-dimensional vector in double precision. 
 * It's important to note that some properties may be derived from arguments a 
 * vector was constructed with, and therefore are only approximations. 
 * Specifically, if: 
 * <pre>

 * {@code v1 = new Vector2D(p1, p2);} 
 * {@code v2 = new Vector2D(v1.getOrigin(), v1.getAngle(), v1.getMagnitude());} 
 * </pre> 
 * then {@code v1.getDestination()} is likely to be unequal to {@code v2.getDestination();} 
 * @author Stephan van Hulst 
 * @version 29-11-11 
 */  
final class Vector2D {  
  
  private final Point2D origin, destination;  
  private final double angle, magnitude;  
  
  Vector2D(Point2D origin, Point2D destination) {  
    double x = destination.x() - origin.x();  
    double y = destination.y() - origin.y();  
  
    this.origin      = origin;  
    this.destination = destination;  
    this.angle       = Math.atan2(y,x);  
    this.magnitude   = Math.hypot(x,y);  
  }  
  
  Vector2D(Point2D origin, double angle, double magnitude) {  
    double x = magnitude * Math.cos(angle);  
    double y = magnitude * Math.sin(angle);  
  
    this.angle       = angle;  
    this.magnitude   = magnitude;  
    this.origin      = origin;  
    this.destination = new Point2D(origin.x() + x,  
                                   origin.y() + y);  
  }  
  
  Point2D getOrigin() {  
    return origin;  
  }  
  
  Point2D getDestination() {  
    return destination;  
  }  
  
  double getAngle() {  
    return angle;  
  }  
  
  double getMagnitude() {  
    return magnitude;  
  }  
  
  Vector2D withOrigin(Point2D origin) {  
    return new Vector2D(origin, this.destination);  
  }  
  
  Vector2D withDestination(Point2D destination) {  
    return new Vector2D(this.origin, destination);  
  }  
  
  Vector2D withAngle(double angle) {  
    return new Vector2D(this.origin, angle, this.magnitude);  
  }  
  
  Vector2D withMagnitude(double magnitude) {  
    return new Vector2D(this.origin, this.angle, magnitude);  
  }  
  
  @Override  
  public String toString() {  
    return origin + " --> " + destination;  
  }  
}

[Dave Elwood]

Interesting Stephan, Memory bloat didn't occur to me. And since this thread started I've butchered about 100 lines of useless code and it still works.

To Winston : as a matter of fact I really do have a VectorArrow class as a member of the MyVector class, just as you describe.

Thanks to all who answered.