Fields vs method arguments

Question

I just started writing some new class and it occurred to me that I was adding a lot of method arguments that are not strictly needed. This is following a habit to avoid having state in classes that is specific to some method call, rather than being general configuration or dependencies of the class.

Doing so means that a lot of methods that could have no arguments end up with one, two or three.

I'd like to hear your opinions on what you think of this tradeoff, and how you go about deciding which approach to take in what situation?

Since code is often easier to understand than English when describing code, I created a little gist that has both variants in it: https://gist.github.com/JeroenDeDauw/6525656

Arguments that are used only as a local variable (not global for the lifetime of the object) should only be in scope as long as it makes sense... I find it very irritating when devs store non-instance state as instance state because it's convenient... Just my opinion though. Makes it hard to see how the execution flow actually is. — Max, Sep 12 '13 at 10:53
Using parameters and letting the class take care of whether or not it may have to change state while executing a method, nicely ties into the ["tell don't ask"](http://martinfowler.com/bliki/TellDontAsk.html) principle. Just bear in mind that Tell don't ask doesn't mean you can't query an object about its state ( [blatant plug](http://programmers.stackexchange.com/a/211189/1324) ) — Marjan Venema, Sep 12 '13 at 18:22

score 7 · Answer 1 · edited Sep 12 '13 at 11:06

7

Using fields co-opts the ability to have multithreading available for the methods that use those fields.

Using fields like that is only slightly better than using globals from a reusability and maintainability standpoint, the key point here is that complex setups need careful and up-to-date documentation about which methods use and/or clobber which fields; something you don't need to do when using arguments.

edited Sep 12 '13 at 11:06

gnat

21,442
29
112
288

answered Sep 12 '13 at 10:50

ratchet freak

25,706
2
62
97

Multhithreading is not relevant for my use case, as it is in PHP. I agree that this approach is bad if the way the fields are used is not trivial. In this case they are only written to (and always written to) from the only public method. I don't anticipate this causing problems. As for fields like this only being slightly better then globals, I'm quite unsure what you mean. Can you elaborate? – Jeroen De Dauw Sep 13 '13 at 16:02

score 7 · Answer 2 · answered Sep 12 '13 at 11:44

In layman's words:

methods should have as few arguments as possible ( Martin's Clean Code )
one of the features of objects is than they can (and should) have a state
non-static methods that don't operate on the object's state, i.e. receive everything as parameters, are not cohesive
non-cohesive methods might as well be made static and grouped in an utility class

Again, in my humble opinion non-cohesive methods belong to an utility class and not to a class with a domain name.

Heinzi · Accepted Answer · 2013-09-12T12:32:02.650

Since the only externally visible method of your example is updateTable, I think it is fine to use fields instead of method parameters.

If this is part of a more generic class (e.g. TableTools), I would move the helper methods that need the state into a hidden inner class.

Pseudo-code example:

class TableTools {
    ...
    public void updateTable(currentTable, newTable) {
        TableUpdater u = new TableUpdater(schemaModifier, currentTable, newTable);
        u.removeRemovedFields();
        u.addAddedFields();
     }

     private class TableUpdater { ... }
}

That way, you avoid fields that are used by only one public method. In addition, the code is thread-safe in the sense that every call to updateTable uses its own copy of TableUpdater and, thus, of TableUpdater's instance variables.

score 1 · Answer 4 · answered Sep 12 '13 at 14:52

Don't use fields in the current case! Two "threads" using the object at the same time will seriously confuse each other. They don't have to be real, separate threads either (hence the quotes). If you set up the object for one table, then call a method that uses it for another table, then try to use the original set up, you have a problem. Stick with parameters for the moment.

What you want to do here is create a new updater class that is used in only one case. The original class could have a method to create an instance whenever needed. The new class would have fields. You have the best of both worlds. Sometimes it's simpler just to stick with the parameters, but in your example you're already getting to where a separate class would be better.

score 0 · Answer 5 · answered Sep 12 '13 at 13:26

I think, that the choosing should be according to the real situation, as you see it. If an item belongs to an instance, it should be seen as its field. If the item is external to the instance, it should be passed as a method parameter.

We should be guided in this case not by the effectivity (the difference is insignificant anyway), or (God save!) easiness of typing, but by the understandability and naturalness of the code.

score 0 · Answer 6 · answered Sep 12 '13 at 15:31

In my opinion if you are writing code that does something to something then it should take parameters that define what things it should do them to and its name should define as far as possible what it does to it.

If you are writing code that packages up an action to be performed on something then you should wrap the things it should be performed on in an object and pass them to your thing that does something.

This action then becomes a kind of meta-description of the calls to the methods of the first that you can then perhaps perform at a later date by queueing it up or even decide not to do it at all for some reason.

So your question translates into is it an Action or a Function? An Action could be postponed or cancelled and therefore should encapsulate that which it acts upon. A Function happens immediately so there is no need top preserve its parameters.

You can undo and Action but not a Function.

Fields vs method arguments

6 Answers6