Function Parameter best practice

Question

I have question regarding the use of function parameters.

In the past I have always written my code such that all information needed by a function is passed in as a parameter. I.e. global parameters are not used.

However through looking over other peoples code, functions without parameters seem to be the norm. I should note that these are for private functions of a class and that the values that would have been passed in as paramaters are in fact private member variables for that class.

This leads to neater looking code and im starting to lean towards this for private functions but would like other peoples views.

E.g.

Start();  
Process();  
Stop();

is neater and more readable than:

ParamD = Start(paramA, ParamB, ParamC);  
Process(ParamA, ParamD);  
Stop(ParamC);

It does break encapsulation from a method point of view but not from a class point of view.

Answer 1

As you mentioned, there's a trade-off between them. There's no hard rule for always preferring one to another. Minimizing the scope of variables will keep their side effect local, the code more modular and reusable and debugging easier. However, it can be an overkill in some cases. If you keep your classes small (which you should do) then the shared variable would generally make sense. However, there can be other issues such as thread safety that might affect your choice.

Answer 2

Generally, it's better to use parameters. Greatly increases the ability to use patterns like dependency injection and test-driven design.

If it is an internal only method though, that's not as important.

Answer 3

Not passing the object's own member attributes as parameters to its methods is the normal practice: effectively when you call myobject.someMethod() you are implicitly passing the whole object (with all its attributes) as a parameter to the method code.

Answer 4

I generally agree with both of Mehrdad and Mufasa's comments. There's no hard and fast rule for what is best. You should use the approach that suits the specific scenarios you work on bearing in mind:

• readability of code
• cleanliness of code (can get messy if you pass a million and one parameters into a method - especially if they are class level variables. Alternative is to encapsulate parameters into groups, and create e.g. a struct to whole multiple values, in one object)
• testability of code. This is important in my opinion. I have occassionally refactored code to add parameters to a method purely for the purpose of improving testability as it can allow for better unit testing

Answer 5

This is something you need to measure on a case by case basis.

For example ask yourself if you were to use parameter in a private method is it ever going to be reasonable to pass a value that is anything other than that of a specific property in the object? If not then you may as well access the property/field directly in the method.

OTH you may ask yourself does this method mutate the state of the object? If not then perhaps it may be better as a Static and have all its required values passed as parameters.

There are all sorts of considerations, the upper most has to be "What is most understandable to other developers".

Answer 6

I don't pass the object's state to the private methods because the method can access the state just like that.

I pass parameters to a private method when the private method is invoked from a public method and the public method gets a parameter which it then sends to the private method.

Public DoTask( string jobid, object T)
{
 DoTask1(jobid, t);
 DoTask2(jobid, t);
}

private DoTask1( string jobid, object T)
{
}

private DoTask2( string jobid, object T)
{
}

Answer 7

In an object-oriented language it is common to pass in dependencies (classes that this class will communicate with) and configuration values in the constructor and only the values to actually be operated on in the function call.

This can actually be more readable. Consider code where you have a service that generates and publishes an invoice. There can be a variety of ways to do the publication - via a web-service that sends it to some sort of centralized server, or via an email sent to someone in the warehouse, or maybe just by sending it to the default printer. However, it is usually simpler for the method calling Publish() to not know the specifics of how the publication is happening - it just needs to know that the publication went off without a hitch. This allows you to think of less things at a time and concentrate on the problem better. Then you are simply making use of an interface to a service (in C#):

// Notice the consuming class needs only know what it does, not how it does it
public interface IInvoicePublisher {
  pubic void Publish(Invoice anInvoice);
}

This could be implemented in a variety of ways, for example:

public class DefaultPrinterInvoicePublisher
  DefaultPrinterInvoicePublisher _printer;
  public DefaultPrinterInvoicePublisher(DefaultPrinterFacade printer) {
    _printer = printer
  }
  public void Publish(Invoice anInvoice) {
    printableObject = //Generate crystal report, or something else that can be printed
    _printer.Print(printableObject);
  }

The code that uses it would then take an IInvoicePublisher as a constructor parameter too so that functionality is available to be used throughout.

Answer 8

There's nothing wrong in principle with having functions access object fields, but the particular example you give scares me, because the price of simplifying your function calls is that you're obfuscating the life cycle of your data.

To translate your args example into fields, you'd have something like:

void Start() {
    // read FieldA, FieldB, and FieldC
    // set the value of FieldD
}

void Process() {
    // read FieldA and do something
    // read FieldD and do something
}

void Stop() {
    // read the value of FieldC
}

Start() sets FieldD by side effect. This means that it's probably not valid to call Process() until after you've called Start(). But the code doesn't tell you that. You only find out by searching to see where FieldD is initialized. This is asking for bugs.

My rule of thumb is that functions should only access an object field if it's always safe to access that field. Best if it's a field that's initialized at construction time, but a field that stores a reference to a collaborator object or something, which could change over time, is okay too.

But if it's not valid to call one function except after another function has produced some output, that output should be passed in, not stored in the state. If you treat each function as independent, and avoid side effects, your code will be more maintainable and easier to understand.