Prefer class members or passing arguments between internal methods?

Question

Suppose within the private portion of a class there is a value which is utilized by multiple private methods. Do people prefer having this defined as a member variable for the class or passing it as an argument to each of the methods - and why?

On one hand I could see an argument to be made that reducing state (ie member variables) in a class is generally a good thing, although if the same value is being repeatedly used throughout a class' methods it seems like that would be an ideal candidate for representation as state for the class to make the code visibly cleaner if nothing else.

Edit:

To clarify some of the comments/questions that were raised, I'm not talking about constants and this isn't relating to any particular case rather just a hypothetical that I was talking to some other people about.

Ignoring the OOP angle for a moment, the particular use case that I had in mind was the following (assume pass by reference just to make the pseudocode cleaner)

int x
doSomething(x)
doAnotherThing(x)
doYetAnotherThing(x)
doSomethingElse(x)

So what I mean is that there's some variable that is common between multiple functions - in the case I had in mind it was due to chaining of smaller functions. In an OOP system, if these were all methods of a class (say due to refactoring via extracting methods from a large method), that variable could be passed around them all or it could be a class member.

Answer 1

If the value is a property of the class, keep it in the class. Otherwise, keep it outside. You don't design your class methods first. You design its properties first. If you did not think of putting that property inside the class in the first place, there is probably a reason for that.

The worst thing you can do, in terms of scalability, is changing your code for convenience. Sooner, rather than later, you'll find your code is bloated and duplicated. However, I must admit. Sometimes, I break this rule. Convenience is just so damn appealing.

Answer 2

If you don't actually need to hold the state between invocations (and apparently you don't, or you wouldn't ask the question) then I would prefer the value be an argument, rather than a member variable, because a quick glance at the method's signature tells you that it uses the argument, while it's a little harder to tell right away what member variables are used by the method. It is also not always quick to determine what private member variables are for.

So normally I wouldn't agree that code using member variables is visibly cleaner, but if the method signatures are getting out of hand I might make an exception. It's a worthwhile question, but not something the project will hinge on in any case.

Answer 3

Thanks for asking the question, I wanted to ask this also.

When I was thinking about this, there are some advantages in why to pass it as argument

• it's easier to test -> easier to maintain (related to last point)
• has no side effects
• it's easier to understand

I clearly see your point for example - one is parsing Excel document (using POI library for example) and instead of passing row instance to every method that needs to work with that row, author has member variable currentRow and works with it.

I'd say there should be a name for this anti-pattern, is it? (not listed here)

Answer 4

Perhaps you should extract a new class containing all the methods sharing that value. Of course the highest-level method will be public in the new class. It may be helpful to expose that method for testing.

If you have two or more temporaries that are always passed together, then you should almost certainly extract a new class.

Answer 5

Do people prefer having this defined as a member variable for the class or passing it as an argument to each of the methods - and why?

If I understand what you're asking: The methods in a class are by definition privy to the implementation details, so I'd have no qualms about using any member directly from any method.

On one hand I could see an argument to be made that reducing state (ie member variables) in a class is generally a good thing...

There's nothing wrong with declaring a private static final to define constants. The compiler will be able to use the value in considering some optimizations, and being constants, they don't really add state to the class.

although if the same value is being repeatedly used throughout a class' methods...

Being able to refer to it symbolically (e.g., BLRFL_DURATION) and not having to add extra arguments to your methods will make your code more readable and therefore more maintainable.

Answer 6

I really encourage the separation of concerns in the implementation of code. In a simplest case of separation, you have classes that contain data and classes that implement processes. For classes that implement processes, they have no state data. They just implement the processing of data into new data and/or events. For classes that store data, their methods are limited to the getting, setting and the slicing of the data for public consumption.

In this approach, this ensures that in your dosomething_xxx() methods are static, and can be easily unit tested. If the methods need to trigger additional methods on other objects, those objects are passed in as one of the parameters to the method. This approach is called inversion of control and creates code that is generally decoupled, which has the added advantage of making future changes more contained in their impact.

Answer 7

if the value does not change, it is by definition a constant and should be encapsulated inside the class. In such case, it is not considered to affect the state of an object. I don't know your case but I think of something such as PI in trigonometry. If you try to pass an argument of such a constant, you could be exposing the result to error if the client passes the wrong value or a value not with the same accuracy as the method(s) expect.