Should private functions modify field variable, or use a return value?

Question

I'm often running into the same trail of thought when I'm creating private methods, which application is to modify (usually initialize) an existing variable in scope of the class. I can't decide which of the following two methods I prefer.

Lets say we have a class Test with a field variable x. Let it be an integer. How do you usually modify / initialize x ?

a) Modifying the field directly

private void initX(){
    // Do something to determine x. Here its very simple.
    x = 60;
}

b) Using a return value

private int initX(){
    // Do something to determine x. Here its very simple.
    return 60;
}

And in the constructor:

public Test(){
    // a)
    initX();
    // b)
    x = initX();
}

I like that its clear in b) which variable we are dealing with. But on the other hand, a) seems sufficient most of the time - the function name implies perfectly well what we are doing!

Which one do you prefer and why?

Thank for your answers guys! I'll make this a community wiki as I realize that there is no correct answer to this.

Answer 1

Actually only a) method behaves as expected (by analyzing method name). Method b) should be named 'return60' in your example or 'getXValue' in some more complicated one.

Both options are correct in my opinion. It all depeneds what was your intention when certain design was choosen. If your method has to do initialization only I would prefer a) beacuse it is simplier. In case x value is also used for something else somewhere in logic using b) option might lead to more consistent code.

You should also always write method names clearly and make those names corresponding with actual logic. (in this case method b) has confusing name).

Answer 2

I usually prefer b), only I pick a different name, like computeX() in this case. A few reasons for why:

• if I declare computeX() as protected, there is a simple way for a subclass to influent how it works, yet x itself can remain a private field;
• I like to declare fields final if that's what they are; in this case a) is not an option since initialization has to happen in compiler (this is Java-specific, but your examples all look Java as well).

That said, I don't have a strong preference between the two methods. For instance, if I need to initialize several related fields at once, I will usually pick option a). That, though, only if I cannot or don't want for some reason, to initialize directly in constructor.

Answer 3

For initialization I prefer constructor initialization if it's possible, public Test():x(val){...}, or write initialization code in the constructor body. Constructor is the best place to initialize all the fields (actually, it is the purpose of constructor). I'd use private initX() approach only if initialization code for X is too long (just for readability) and call this function from constructor. private int initX() in my opinion has nothing to do with initialization(unless you implement lazy initialization,but in this case it should return &int or const &int) , it is an accessor.

Answer 4

I would prefer option b), because you can make it a const function in languages that support it.

With option a), there is a temptation for new, lazy or just time-stressed developers to start adding little extra tasks into the initX method, instead of creating a new one.

Also, in b), you can remove initX() from the class definition, so consumers of the object don't even have to know it's there. For example, in C++.

In the header:

class Test {
private:    int X;
public:    Test();
...
}

In the CPP file:

static int initX() { return 60; }

Test::Test() {
    X = initX();
}

Removing the init functions from the header file simplifies the class for the people that have to use it.

Answer 5

Neither?

I prefer to initialize in the constructor and only extract out an initialization method if I need a lot of fields initialized and/or need the ability to re-initialize at another point in the life time of an instance (without going through a destruct/construct).

More importantly, what does 60 mean?

If it is a meaningful value, make it a const with a meaningful name: NUMBER_OF_XXXXX, MINUTES_PER_HOUR, FIVE_DOZEN_APPLES, SPEED_LIMIT, ... regardless of how and where you subsequently use it (constructor, init method or getter function).

Making it a named constant makes the value re-useable in and of itself. And using a const is much more "findable", especially for more ubiquitous values (like 1 or -1) then using the actual value.

Only when you want to tie this const value to a specific class would it make sense to me to create a class const or var, or - it the language does not support those - a getter class function.

Another reason to make it a (virtual) getter function would be if descendant classes need the ability to start with a different initial value.

---

Edit (in response to comments):

For initializations that involve complex calculations I would also extract out a method to do the calculation. The choice of making that method a procedure that directly modifies the field value (a) or a function that returns the value it should be given (b), would be driven by the question whether or not the calculation would be needed at other times than "just the constructor".

If only needed at initialization in the constructor, I would prefer method (a).

If the calculation needs to be done at other times as well, I would opt for method (b) as it also makes it possible to assign the outcome to some other field or local variable and so can be used by descendants or other users of the class without affecting the inner state of the instance.

Answer 6

@Frederik, if you use option b) and you have a LOT of field variables, the constructor will become a quite unwieldy block of code. Sometimes you just can't help but have lots and lots of member variables in a class (example: it's a domain object and it's data comes straight from a very wide table in the database). The most pragmatic approach would be to modularize the code as you need to.