Pergunta

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

Foi útil?

Solução

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.

Outras dicas

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.

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.

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.

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.

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.

Licenciado em: CC-BY-SA com atribuição
scroll top