public final variables

[Razanir]

Is it considered bad coding practice to have public instance variables, even if they're final?

So something like:


public class MyClass {
public final int myInt;
public final double myDouble;

//other methods and variables
}

[Douglas]

Typically there's no point in having one copy of a final variable per object, as it would require some rather unusual coding to make them have different values, so they should probably be static as well.

Public static final variables are quite common as a way to implement named constants.

[Razanir]

Typically there's no point in having one copy of a final variable per object, as it would require some rather unusual coding to make them have different values, so they should probably be static as well.

Public static final variables are quite common as a way to implement named constants.

Actually, I'll use a few examples:

1) Suppose I have an Address class that's really just a bag-o'-Strings. If I never anticipate needing to change the values, is it okay to make everything public final variables? I would think "Yes," because it sounds similar to what I know about structs in C.

2) Let's change it to a bag-o'-primitives, except it also contains an Address (which as stated above, is a fancy bag-o'-Strings). What about now?

3) Now suppose I have a more complicated class, BagOfPrimitives. Like with the Address class, I never anticipate needing to change the values after instantiation. But this time, I also need a few methods that output the results of a few calculations. Would it (also) be reasonable here to have public final variables?

[Mastikator]

If you're never going to change the variables then there's no need to make objects. Just use
static class Stuff
{
public static final string data1 = "foo";
public static final string data2 = "bar";
}

[Holocron Coder]

In the third case, it's less useful, since the "final" just refers to the object itself. All of its values would be changeable through normal methods (setX, etc). Thus, the "final" is essentially meaningless.

[Razanir]

If you're never going to change the variables then there's no need to make objects. Just use
static class Stuff
{
public static final string data1 = "foo";
public static final string data2 = "bar";
}

I think you're missing the point. I'd still need multiple addresses. It's just that once instantiated, I never need to change the instance variables.

[Drumbum42]

Why are you making the variables available in the first place? Generally people use "getters" for this sort of thing. like:


var=object.getAddress();
instead of:

var=object.address;

Assuming that it's either a primitive or not a direct object reference to the original, it's just as good as making the variable a final because the value can not be changed. The advantage is that if one day "address" needs to be changed, you can whip up a quick method instead of changing all your public finals to public.

so:


int address;

public int getAddress(){
return address;
}

[Capt Spanner]

I would always make variables private, unless there's a very good reason not to.

I'll give a real life example that happened to me:

I was keeping track of physics objects. They all had a mass and a velocity and a momentum:



class Physics {
public:
const float mass;
Vec3 velocity;
Vec3 getMomentum() const { return mass * velocity; }
void giveMomentum( const Vec3& momentum ) {
Vec3 newMomentum = momentum + getMomentum();
velocity = newMomentum / mass;
}
// Other stuff.
};


Now, speed was of the essence, as was size (for cache coherency reasons). However, the giveMomentum() call was the most common way of interacting with this object. That call, with it's expensive floating point divide, was where the code spent most of its time, as we'd call giveMomentum() tens or hundreds of time per object per frame, while getVelocity() was only called once per frame, as it turned out.

So I refactored it and then I had to go through the ENTIRE THING changing EVERY REFERENCE to Physics::velocity to Physics::getVelocity(), and so on, in 1,500 lines of code across around 20 source files. It took a good 90 minutes in all.

On the other hand, if I'd written it like this:



class Physics {
private:
float mass;
Vec3 velocity;
// Other attributes.
public:
float getMass() const { return mass; }
Vec3 getVelocity() const { return velocity; }
Vec3 getMomentum() const { return mass * velocity; }
void giveMomentum( const Vec3& momentum ) {
Vec3 newMomentum = momentum + getMomentum();
velocity = newMomentum / mass;
}
// Other stuff.
};


I would have only needed to change two functions in one source file:



class Physics {
private:
float mass;
Vec3 momentum;
// Other attributes.
public:
float getMass() const { return mass; }
Vec3 getVelocity() const { return momentum / mass; }
Vec3 getMomentum() const { return momentum; }
void giveMomentum( const Vec3& additionalMomentum ) {
momentum += additionalMomentum;
}
// Other stuff.
};


No further changes are needed.

Having said that, there are occasions where this sort of thing is much less likely to happen.

For example, the Vec3 class used above was:



class Vec3 {
public:
float x , y, z;
// A whole load of operators and functions.


Here's it's fine, because: 1. x, y, z are orthogonal (i.e. changing one does not affect the other two); 2. A vector like this is very simple and never going to change.