views:

86

answers:

2

Context

From The Pragmatic Programmer:

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

Questions

  • How is that statement reconciled with directly setting a private member variable's value throughout a class in multiple places?
  • Does it matter as there can be no external dependencies on the value?
  • Is it duplication to directly change private member variables that have public accessors in other places besides the accessor?

Example

Consider the following code:

public class Line {
  private boolean changed;
  private double length;
  private Point start;
  private Point end;

  public Line( Point p1, Point p2 ) {
    this.start = p1;
    this.end = p2;
    this.changed = true;
  }

  public void setStart( Point p ) { this.start = p; this.changed = true; }
  public void setEnd( Point p ) { this.end = p; this.changed = true; }
  public Point getStart() { return this.start; }
  public Point getEnd() { return this.end; }

  public double getLength() {
    if( this.changed ) {
      this.length = start.distanceTo( end );
      this.changed = false;
    }

    return this.length;
  }
}

Even though the changed variable is never exposed (through public acccessors or otherwise), the same line of code is essentially repeated four times: this.changed = true (thrice) and this.changed = false (once). Similarly, the assignment of this.start and this.end happens multiple times. As opposed to:

  public Line( Point p1, Point p2 ) {
    setStart( p1 );
    setEnd( p2 );
  }

  public void setStart( Point p ) { this.start = p; dirty(); }
  public void setEnd( Point p ) { this.end = p; dirty(); }

  public double getLength() {
    if( isDirty() ) {
      setLength( getStart().distanceTo( getEnd() ) );
      clean();
    }

    return this.length;
  }

The updated code is quite similar, but the duplication of all assignments is removed (presume dirty() and clean() use accessors). (There is a duplicated call to dirty() in the constructor that was not there before due to reusing the accessor methods for assignment.)

The question is not about whether this.changed = true is more readily understood as dirty().

Clarification

The question is about whether this.variable = value is a "piece of knowledge" and should therefore have a "single, unambiguous, authoritative representation" that is used consistently: a corresponding accessor. Thus the general case:

public class C1 {
  private Object v;

  public C1() {
    this.v = new C1();
  }

  public void m1() {
    this.v = new String();
  }

  public void m2() {
    System.out.println( this.v );
  }
}

versus:

public class C2 {
  private Object v;

  public C2() {
    setV( new C2() );
  }

  public void m1() {
    setV( new String() );
  }

  public void m2() {
    System.out.println( getV() );
  }

  private void setV( Object o ) { this.v = o; }
  private Object getV() { return this.v; }
}

In C1, the variable v is directly assigned in multiple places. In C2, the variable v is is directly assigned in a single spot. Even though, in both cases, v is completely private, does the C1 implementation duplicate a "piece of knowledge"?

+1  A: 

A method like dirty() has more semantic meaning than this.changed = true. If you ever decide you want to handle the dirty-tracking a different way, you only have to change one place---and from the point of view of other code (even if all in the same class), it's still more meaningful (and easier for the reader to grasp).

In short, I recommend using dirty() instead of this.changed = true.

Chris Jester-Young
+3  A: 

How is that statement reconciled with directly setting a private member variable's value throughout a class in multiple places?

There is a single private member variable. Therefore there is a single representation. The statements that change this representation are not representations themselves. Having multiple statements that access / change the representation is not the same as having multiple representations.

Does it matter as there can be no external dependencies on the value?

No.

Is it duplication to directly change private member variables that have public accessors in other places besides the accessor?

No.

That doesn't necessarily mean that it is a good idea to do it though.

In your example, the choice is between accessing and updating a "dirty" flag directly or doing this via light-weight private methods. IMO, this boils down to a value judgment as to which approach gives you more readable code. And my feeling is that there is little difference between the two approaches, at least in this case. In other cases, there could be a stronger case for using internal methods to access / update private state that is never exposed.

If the state needs to be exposed outside of the class then there is a strong case for declaring the variables private and providing getters and setters for other classes to use. And if those getters and setters have been declared, then you can make a (weaker) case that the class itself should use them.

For those people who are concerned about the efficiency or otherwise of getters and setters in Java, the chances are that it will make no difference to performance. The JIT compiler in a modern JVM will almost certainly inline methods like clean(), dirty() and isDirty() resulting in machine instructions are equivalent to the case where you get and set the variables directly. Indeed, the latest JIT compilers will even inline non-final public methods when they can deduce that the methods don't need to be dispatched.

Stephen C