The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Dear Amateur Programmers ...

Dear Amateur Programmers,

It is wonderful to see so many people playing in the field of software development. An English teacher once remarked that people should only be allowed to use three exclamation marks in their entire life. I am about to use my second in the following request ...

KINDLY STOP DIRECTLY ACCESSING CLASS-SCOPE VARIABLES!

You know who you are; you write (Java) code like this:

public class Person {
  // A class-scope variable.
  //
  private int age;

  public Person( int age ) {
    // Directly modifying the class-scope variable:
    //
    this.age = age;
  }

  public void print() {
    // Directly using the class-scope variable:
    //
    System.out.println( "My age = " + age );
  }

  public void birthday() {
    // Direct usage and modification:
    //
    this.age++;
  }
}

I beg you to stop writing code like the above, and to smack it down wherever it roams. Especially if written by so-called senior (or professional) software developers and engineers. It is not correct. It is not good. It leads to bug-ridden, brittle, unmaintainable, spaghetti-esque software.

Always use accessors. Always. Always. Always. Like this:

public class Person {
  private int age;

  public Person( int age ) {
    setAge( age );
  }

  public void print() {
    System.out.println( "My age = " + getAge() );
  }

  public void birthday() {
    setAge( getAge() + 1 );
  }

  private int getAge() {
    return this.age;
  }

  private void setAge( int age ) {
    this.age = age;
  }
}

When you consistently use accessors source code gains the following advantages:

1. Can place a breakpoint (or debug statement) in the set method. This tells you exactly when the value changes, and to what value the variable is being changed.

2. Can constrain the variable to specific values. Since the value is being set in a single location throughout the entire class, you can place limits on acceptable values.

3. Lazy initialization (which can significantly improve application start-up time). For example:

public class LazyInitializationExample {
  private Object lazyInit;

  public Object getLazyInit() {
    if( this.lazyInit == null ) {
      this.lazyInit = createLazyInit();
    }

    return this.lazyInit;
  }

  // This allows subclasses to return a different subclass
  // of Object. Very powerful mechanism. Thank me later.
  //
  protected Object createLazyInit() {
    return new Object();
  }
}

4. It reduces coupling between sub-classes and super-classes. When subclasses access inherited variables  through accessors, the underlying implementation of those variables can change without adversely affecting its subclasses. This helps eliminate the problem where a change in the superclass causes an undesired ripple effect throughout subclasses.

I don't really understand this point, either, but trust me that it's a good thing. ;-)

5. Like adding constraints, you can: add validation code, assert for specific values, readily apply unit testing, and so forth.

6. Can add undo and redo functionality. Since the only place the value changes sits in a single location, its easy to track those changes in a history.

7. Reap the rewards of Aspect Oriented Programming (accessors as insertion points).

8. Encapsulation. If the business rules for a variable change, you can update the accessors to provide the same capability as before the change, making it easier for you to respond to the new business rules.

See also: http://www.ibm.com/developerworks/java/library/ws-tip-why.html

Regarding optimization:

(1) Let the compiler do it.
(2) Modern compilers might inline the accessor call.
(3) Modern Just-In-Time compilers will inline the call.
(4) Run a profiler to find your bottlenecks.
(5) Research optimization techniques that do not involve eliminating accessor methods.
Dave Jarvis Send private email
Thursday, April 10, 2008
 
 

Thursday, April 10, 2008
 
 
Can't say I agree with you, Dave. Does that make me an amateur programmer? :-)

Your explanation is good but it reeks of "consistency over everything else". Better simply to acknowledge that this a topic debated by many very experienced Java programmers without agreement.
Steve McLeod Send private email
Thursday, April 10, 2008
 
 
I heartily disagree with this entire thesis. Not for reasons of performance, but for the reason that simple code is easier to maintain. It is simply easier to use this.x than getX() accessor for simple variables.

This is not difficult to change later. If we want to add variable constraints, lazy initialization, validation, undo/redo functionality, or AOP accessor hooks, we can always change the accesses to getters and setters, at that point. Since the class variables are of course private, the only code that has access to them is in that class, so it will be easy to change it to use accessors. That is the idea behind private variable accessibility. There is no reason to add this functionality before it is necessary, since it usually doesn't need to be added. Once again, this is encapsulation at the class level, rather than at the variable level.

As to setting breakpoints, modern IDEs can set breakpoints at variable and field access.

Reducing coupling. If you don't understand this point, I certainly don't ;-). A private field isn't accessible to subclasses, so presumably the coupling is already reduced.

In short, I don't see the need to continue to advocate using getters and setters on private fields. This is part of the reason Java code ends up as cumbersome and hard to work with.
Avi Rosenschein Send private email
Thursday, April 10, 2008
 
 
getters and setters on non externally visible stuff is just codebloat.

And frankly, I'm starting to think that getters and setters and anything is a waste of effort -- if you later on need to impose constraints you can usually do it by dropping proxies in to replace the actual variables.

Classes end up being one of two types; a data storage system (in which case everything ends up playing with the members anyway -- so it might as well have them public) or an actual entity, in which case it's going to have a teeny number of clearly responsibilities and all the members are internal.

In the latter case, faffing with getters and setters is just going to make the verbs more obfuscated.
Katie Lucas
Thursday, April 10, 2008
 
 
I am a big fan of best practices, but I'm going to have to disagree with you on this one. It's not that the individual points you mention are wrong but rather that I am not aware of any best practice that is meant to be applied blindly. You're supposed to use your judgment to decide whether they're appropriate for your specific case and there are always exceptions to the rule.

A similar best practice that becomes harmful when applied excessively is "comment your code" :)

Applying best practices blindly leads to chronic over-engineering. Specifically, many Java programmers have been blamed for using Design Patterns or building frameworks for use-cases did not actually warrant it (just for the sake of trying them out). Einstein said it best: "Make everything as simple as possible, but not simpler"

I believe the book Refactoring by Martin Fowler actually strikes a perfect balance in this regard. To paraphrase: "Design software as simple as possible and refactor as new requirements warrant added complexity." There is a cost to any design paradigm (not necessarily performance-wise) and you should avoid paying it as long as humanly possible.

That's the general answer. On a more specific level I would say the following:

1) You should abstract access to class fields the user needs to access. Think long and hard whether the user really needs to access them :) If you're designing a library you can add these methods on-demand as use-cases warrant them.

2) Abstracting access to class fields does not necessarily mean using accessors/mutators. On a certain level these actually defeat class encapsulation.

Ideally you want to introduce use-case driven methods instead of exposing the data directly. For example, if you're designing a car navigation system, you would prefer "car.eta(destination)" to "car.getSpeed()", "car.getLocation()" and calculating the ETA externally. The former lets you modify the underlying data far more easily and leads to code which is easier to read.

4) This ties in to an excellent saying by Joshua Bloch: "When in doubt, leave it out".
http://www.infoq.com/presentations/effective-api-design

You should minimize the amount "conceptual weight" you introduce into your designs.


PS: I believe points 4 and 8, 2 and 5 amount to the same thing.
Gili Send private email
Thursday, April 10, 2008
 
 
I confess this is one practice I've never quite seen the payoff from.
Codeless
Thursday, April 10, 2008
 
 
+1 for Avi.

I like my code simple as long as possible. The use of accessors is certainly not stupid, but I would do it when it has become necessary.

A nice alternative is provided by Ada, where one can redefine the assignment routine. This provides the tight control of accessors and the simplicity of direct access. Ada was an improvement over later languages 8-).
Karel Thönissen Send private email
Thursday, April 10, 2008
 
 
>> It is wonderful to see so many people playing in the field of software development. <<

I am glad that you also enjoy playing in this field. As you grow and mature, you will learn that there is no such thing as a "best practice" independent of context.

You will also learn that discussing your technical biases with the Internet is a very peculiar form of masochism.
Mark Pearce Send private email
Thursday, April 10, 2008
 
 
You ever notice how someone new to patterns over uses them ... everything has to match one and you end up with ugly cases where they're crammed into odd corners of the program.

This post looks like the textbook description of why encapsulation is good (which I agree with), but encapsulation at the class level is "good enough" in every case I've ever seen.

In any case, your one valid point is that it's useful to have the debugger stop when a variable changes ... that's called a watchpoint and is available in any modern debugger.  Even an amateur should know that;)
Steve Moyer Send private email
Thursday, April 10, 2008
 
 
Leave optimization to the compiler?

Cop out.

Things go wrong with compiler optimization. Stealthily. And when you realize something's broken, the only solution is... turn off compiler optimization!

And I'll use as many exclamation points as I feel are called for, whenever I feel they're called for.

The hallmark of a seasoned professional is he knows the difference between theory and reality. I take best practices under advisement. But I submit to empirical knowledge without question.
Rowland
Thursday, April 10, 2008
 
 
Another point.

If the compiler inlines your assignment method you will have a hard time inserting a breakpoint therein.
Rowland
Thursday, April 10, 2008
 
 
Having a bad day, are we? ;)

A better advice IMHO, is to avoid mutable objects completely. If you need value objects (Such as representations of database records), make them immutable and have "accessors" return a clone instead of altering state. Yes, there is a slight performance overhead, but for most applications it's a drop in the ocean.
Troels Knak-Nielsen Send private email
Thursday, April 10, 2008
 
 
I have to agree with everyone else. If this is the biggest worry you have to deal with then you are very lucky indeed. Encapsulation at the class level is enough. You have enough people arguing that getters/setters are a waste of time in general and opt for using public fields instead. Don't give them fuel for their fire by insisting that they use setters for internal class access as well.

Go have a beer and take a rest. You are clearly under too much pressure.
dood mcdoogle
Thursday, April 10, 2008
 
 
Dear Dave Jarvis, please give us more posts
about programming "best" practices.
I am going to call this one "Exhibit A".
Object Hater
Thursday, April 10, 2008
 
 
I think the OP has misunderstood the idea of accessor methods as a means of enforcing encapsulation.  The referenced developerworks article is advocating the use of accessor methods as opposed to direct access to member variables from _other_ classes.

IMO, the important thing from an OO perspective is to ensure that member variables are private and not accessible from subclasses.    This ensures that subclasses do not directly access superclass member variables which introduces strong coupling.
Joe
Thursday, April 10, 2008
 
 
Lot of (well-deserved) negativity in the responses here.  I think we should try to find something positive to say about OP's post.

He's used up 1/3 of his life-time supply of exclamation points...

Will
William Dowling Send private email
Thursday, April 10, 2008
 
 
I think what the OP was basically saying amounted to 'if you only change a private field state via an accessor method, you can put a breakpoint on that method and get a break whenever it is accessed and the field value will change'. In which case my answer is, why not just put a watch (or whatever the Eclipse equivalent is) on the variable?

Now if your accessor has a side-effect that you want to take advantage of in private code, then just call the public accessor. Why have unnecessary duplication?

In short - accessors for public use = good, accessors for private use = lard.
Neil Hewitt Send private email
Thursday, April 10, 2008
 
 
There are rare cases when I wish I could mark off a variable,

@LimitToAccessor
int x;

...or something like that. This is typically in cases where some sort of pre- or post-processing is critical and you don't even want the *possibility* of direct access. Thus:

setX(int p_x)
{
  if (p_x == bad)
    throw FatalException()
  x=p_x;
}

...of course, that happens one instance out of a thousand. I've noticed management-types like to take those 1-in-a-1000 cases where XYZ practice is important and apply it system-wide. Please don't do that. It just rots code.
my name is here
Thursday, April 10, 2008
 
 
While in general I disagree with the OP's rule, his reason 2 (preconditions) sometimes makes me adopt that policy. Similar to what My name is here writes, but with assertions.
Daniel_DL Send private email
Thursday, April 10, 2008
 
 
Dear Dave,

// A class-scope variable.
//
private int age;

This is actually an instance variable. A class-scope variable would look like this:

private static int age;

Also, as others have said, I don't see why my classes shouldn't be free to mess around with their own guts. I've never been bitten by any problems with this in six years of miserable Java programming!
John Topley Send private email
Thursday, April 10, 2008
 
 
+100 for my name is here
...
Thursday, April 10, 2008
 
 
I do have a case to support the OP. Recently I had a class with a money value with getter/setter's which were used in some places, in other places the value was accessed directly.

Had to apply a complex formula to the value, only wanted to do this in one place. I put it in the getter and had to chase down and change all the direct access points.

Overall, I agree with most posters here, but there are cases to support the OP.
KooKoo Kachoo Send private email
Thursday, April 10, 2008
 
 
Hi, folks.

Show me the code.

Not a single person who replied provided a source code example where directly accessing an instance variable provides a benefit that outweighs any of the advantages listed.
Dave Jarvis Send private email
Thursday, April 10, 2008
 
 
Dave,

The advantage is in simplicity. No need for any code example to understand that, methinks...

If you're unsure about the benefits of simplicity, just read Joel's rants on architecture astronauts. Good times... :)
Don't Fix What Ain't Broke
Thursday, April 10, 2008
 
 
"Not a single person who replied provided a source code example"

I guess the complexity reduction from directly accessing our fields resulting in code that was too small to see!!!

(I wanted to waste my quota all at once ... now I don't have to worry about it, but I wonder if I'm using too many ellipsii and octothorpes).
Steve Moyer Send private email
Thursday, April 10, 2008
 
 
YAGNI for almost all of your points - sure if I need to do those things then use the pattern you recommend but most of the time I don't so why write the more complex code just in case I *might* need one of these things one day?

"When subclasses access inherited variables  through accessors"

So why not just make your fields private? As the field age in your example so AFAIK it *can't* be accessed by subclasses anyway.
Arethuza
Thursday, April 10, 2008
 
 
Dave, when it's you against the whole world, guess who's going to win?

Given the opportunity cost, there are battles you should fight and battles you shouldn't. This falls into the latter category.
Mark Pearce Send private email
Thursday, April 10, 2008
 
 
>> So why not just make your fields private? As the field age in your example so AFAIK it *can't* be accessed by subclasses anyway. <<

In C# this is true. You need to specify "protected" if you want subtype visibility.
Mark Pearce Send private email
Thursday, April 10, 2008
 
 
Dave Jarvis: "Show me the code.

Not a single person who replied provided a source code example where directly accessing an instance variable provides a benefit that outweighs any of the advantages listed."

Brice? Is that you?

Sorry. Got confused. Brice Richard is usually the one who posts something totally wrong, has dozens of people tell him so and explain why, and then calls them all idiots because they don't agree with his originally wrong ideas.

Are you a cousin or something?
Ken White Send private email
Thursday, April 10, 2008
 
 
"Sorry. Got confused. Brice Richard is usually the one who posts something totally wrong..."

I think that's a bit harsh. The consensus seems to be that what Dave is proposing is usually unnecessary, which is some stretch from totally wrong. It the very worst it's harmless.
John Topley Send private email
Thursday, April 10, 2008
 
 
People always get mean at the bottom of these threads.
my name is here
Thursday, April 10, 2008
 
 
I originally thought this was Brice too, but then I realized the code samples weren't Access;)
Steve Moyer Send private email
Thursday, April 10, 2008
 
 
And because this discussion hasn't deteriorated sufficiently, I'd just like to add that in C#, you'd write:

public class Person
{
  public int Age { get; set; }
}

and then use this.Age or Age interchangeably within the class, foo.Age outside the class, and never worry about directly accessing the underlying variable, because it's not exposed.

And when, in the fullness of time, it turns out that you need to implement something like bounds-checking, you refactor the property:

private int _Age;
public int Age
{
  get { return _Age; }
  set
  {
      if (value < 0)
      {
        throw new AgeException(...);
      }
      _Age = value;
  }
}

which doesn't change the interface to the class.  Of course, at that point you start being able to reference _Age inside the class and bypassing your shiny new bounds-checking logic.  Hammers do occasionally smash thumbs.
Robert Rossney Send private email
Thursday, April 10, 2008
 
 
1.  Disagree, setters for private scope is noise and makes the code significantly less readable.

2.  Code written to support debugging is a poor use of time.  Debugging is not an effective activity.  Debugging is not what makes you a professional programmer.  The ability to automate your unit testing and ensure every line of code of your application has been executed prior to handing it to your customer is what separates an amateur from a professional programmer.
Brian Hart Send private email
Thursday, April 10, 2008
 
 
> KINDLY STOP DIRECTLY ACCESSING CLASS-SCOPE VARIABLES!

Hear, hear. Make them public access so that everyone can have a go ;)
Jussi Jumppanen Send private email
Thursday, April 10, 2008
 
 
"Debugging is not an effective activity"

Didn't have that thread last week?
Arethuza
Friday, April 11, 2008
 
 
>> The ability to automate your unit testing and ensure every line of code of your application has been executed prior to handing it to your customer is what separates an amateur from a professional programmer. <<

Significant amounts of my code have never seen a unit test. For example, all of the code I wrote between 1979 and 2000 wasn't unit-tested. Yet somehow it works perfectly well.

You're making the same mistake as the OP in not understanding that every "best practice" is context-dependent. You can be a fanatic about unit testing or whatever. But don't expect the world to agree with you.
Mark Pearce Send private email
Friday, April 11, 2008
 
 
> The ability to automate your unit testing ... is what separates an amateur from a professional programmer.

Knowing how to do so is one thing, actually doing it is another. I'd argue that where you are on the effort/coverage curve, and the extent to which testing is automated, is a decision for and reflects on the employer and the product, more than the developer.
Christopher Wells Send private email
Friday, April 11, 2008
 
 
"I'd argue that where you are on the effort/coverage curve, and the extent to which testing is automated, is a decision for and reflects on the employer and the product, more than the developer."

SDTimes just had an article on automated Java testing that included some of the top tool vendors warning that there should be a common-sense balance between automated testing and manual testing.  See:

http://www.sdtimes.com/content/article.aspx?ArticleID=31873

and here for their "best practices" sidebar:

http://www.sdtimes.com/content/article.aspx?ArticleID=31872
Steve Moyer Send private email
Friday, April 11, 2008
 
 
Thanks Steve. What do you suppose they mean by the 2nd sentence of best practice #7:

7.    Focus on test design, not tools. Tests should be based on software requirements, not product features.
Christopher Wells Send private email
Friday, April 11, 2008
 
 
And I want to pile-in some more on comprehensive unit testing as a "best practice".

Designing your initial unit test suite to achieve 100% coverage is a dodgy idea. It’s a sure way of creating a test suite weak at finding faults of omission. Also the coverage tool can no longer tell you where your test suite is weak - because it's uniformly weak in precisely the way that coverage can't directly detect. So be careful about code coverage in unit test design. The return on your testing $/£ (in terms of bugs found) is too low.

Suppose you have a manager like Brian who requires some level of code coverage, perhaps 90%, as a "shipping gate". The product isn't done - and you can't ship - until you have 90% coverage. The problem with this approach is that people optimise their performance according to how they’re measured. You can get 90% coverage by looking at the coverage conditions, picking the ones that seem easiest to satisfy, writing quick tests for them, and iterating until done. That's faster than thinking of coverage conditions as clues pointing to weaknesses in the test design. It's especially faster because thinking about test design might lead to 'redundant' tests that don't increase coverage at all. They only find bugs.
Mark Pearce Send private email
Friday, April 11, 2008
 
 
C. Wells ...

I suspect that she's referring to the Rational Unified Process (RUP), where you never code to features, but to requirements.  Then you test to the same requirements and provide traceability both forward and backward.

That's a pretty pathetic description (considering they have a whole book describing it), but it is essentially describes the idea that you perform different testing against different types of requirements (unit testing against software requirements, integration testing against system requirements, etc.).

So it's essentially the waterfall method with extra mechanisms added for requirement traceability and test coverage.
Steve Moyer Send private email
Friday, April 11, 2008
 
 
Mark:

I agree ... start with a reasonable definition of unit tests, but add to the suite every time you find a bug.  It's a nice way to incrementally building a relevant suite of tests.
Steve Moyer Send private email
Friday, April 11, 2008
 
 
> RUP, where you never code to features, but to requirements.

Ah, got it, thank you.

> unit testing against software requirements, integration testing against system requirements

... and, eventually, acceptance testing against the functional specification.

I didn't understand why it seemed to be saying that you shouldn't do acceptance testing; I thought they were implying that the mapping from functional specification to software requirements is perfect, and/or is "proven" by using traceability instead of (not as well as) by running the software ... but more likely it's that such "proving" is meta-testing that's outside the scope of automated testing.
Christopher Wells Send private email
Friday, April 11, 2008
 
 
If someone working for me wrote all their code like this, I would fire them.
Senior Amateur
Saturday, April 12, 2008
 
 
Senior Amateur's attitude is even worse...  Fire someone over their code style?  Maybe have a talk with them and have them change it, but fire them?  Sometimes I find it ridiculous how passionate people in the software industry get over things that in the larger scheme of things are meaningless.  I've seen two people argue over what a local variable should be named for several hours.  Just call it blub and be done with it.

Yes, I know that the above post was likely not serious.

Saturday, April 12, 2008
 
 
One important exception to this is where the entire interface to the class is through the member variables, as in:

class Complex
{
  public double X;
  public double Y;
}

You could throw in copy constructors and toString() members, but for accessing, what's the point of getX()/setX(), getY()/setY()?
.
Sunday, April 13, 2008
 
 
Doesn't Java have some kind of implicit getter/setter construct? Like this in ECMAscript:

private var x:int = 0;
public function get x():int {
    return _x;
}
public function set x( i:int ):void {
    // validation here
    _x = i;
}


It's tremendously useful.
1. Gives most of the benefits of using accessors
2. Allows the rest of the code to use simple language like "x+=5"
3. You can start with a normal variable, and add the accessor functionality later without needing to change places where the variable is being accessed.


I'm not a Java person, but I'd hate to be without this kind of construct.
aph Send private email
Monday, April 14, 2008
 
 
Sorry, above should have read:

private var _x:int = 0;

Obviously _x is the inaccessible copy of x.
aph Send private email
Monday, April 14, 2008
 
 
"Doesn't Java have some kind of implicit getter/setter construct?"

No.
John Topley Send private email
Monday, April 14, 2008
 
 
If it's important why can't accessors be implemented automatically? For example, the compilers should create the get__X and set__X whenever you have a variable X, and when they're used and there is no method of the same name defined in the code. The coders can choose to ignore the accessor until they really need it - and it'd always be there, to be inherited or modified.
AqD Send private email
Monday, April 14, 2008
 
 
Dear Amateur Programmers,

never listen to people who want to sell you a silver bullet that you should "Always. Always. Always." use.

Oh, and never listen to people who want you to "never" listen to other people. :-)

Trust me: Never trust someone who says "trust me".
...
Tuesday, April 15, 2008
 
 
<quote>
I always tell a young man never to use the word 'never'.
</quote>
Christopher Wells Send private email
Tuesday, April 15, 2008
 
 
What the OP is suggesting has in fact some interesting points. While I do not agree that every instance variable  should be wrapped in getter/setters - for the many reasons all the others here already mentioned - I do see the advantages for this practice for special cases.

The example of the OP might not be the best for this, but if I need to check boundaries or other constraints on a instance variable, chances are already high that this variable is already representing a more complex concept than "just a value". In the example, the variable "age" might be replaces by an instance of an "Age"-Class(!) -(or maybe even better a variable "DateOfBirth" of a "Date"-class! but thats beyond the point i try to make here).

So one would be able to correctly add something to that "age" by using the appropriate methods. There would be no confusing what "age = age+1" would mean - is it now one year older? or one day? is it allowed to substract from an "age"? with "age.addYears(1)" there would be no such confusion anymore.

So my point is: think throughoughly(sp?) the way you want to implement your class attriutes. Then think again. And only _then_ implement then. :-)
JK Send private email
Wednesday, April 16, 2008
 
 
That is a really good point. I would call my self a mid level programmer and I have always implemented my code in that manner. I have classes and classes full of many functions/methods. It would not be worth it to go and update each and every instance of that, but I will defiantly use this practice in the future. Thanks for your great advice! I found this article very useful!
Random Thoughts Send private email
Thursday, April 24, 2008
 
 
Dear Dave,

Does the specific languge you use support nested functions inside your methods? Let's assume that it does. Then you could implement getters and setters for your local variables that resides within your methods. Do you? If not, list the arguments why. Then use those argument against your original claim.

My point is that there are no absolute 'good' principles. As many before me already wrote - it's situation dependent.
Stefan Send private email
Tuesday, April 29, 2008
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz