Is it better to have separate functions or add more arguments to a function?

Question

Which is the better style to follow for say, changing the voltage on a 4 channel PSU:

setChannelOneVoltage(voltage)
setChannelTwoVoltage(voltage)
setChannelThreeVoltage(voltage)
setChannelFourVoltage(voltage)

or

setVoltage(channel,voltage)

Answer 1

I would definitely recommend the latter variant.

1. It's more flexible. What if you adopt the code for another device which has not just 4 but 8, 16, 32 or even more channels? You would have to add more and more methods and your code would become more and more convoluted. The code could not be shared between the devices, because you would end up with invalid methods. But when your code leaves the number of channels open, you can use the same code for any device.
2. When you need to set the voltage on multiple channels at the same time (either all to the same value or to different values read from a data structure), you can do this with a simple for-loop. The first version would require to call every single method separately.

By the way, for better code readability, you can replace the channel numbers by constants when you want to. This code might be ambiguous:

setVoltage(2, 4);  // Does this set channel 2 to 4V or channel 4 to 2V?

But this is much more readable:

setVoltage(CHANNEL_2, 4); // OK, channel 2 is now 4V, but what does channel 2 actually do?

This is even more readable:

setVoltage(FLUX_CAPACIATOR_CHANNEL, 4); // Puts 4V on the Flux Capaciator -- Captain Obvious

Answer 2

Under the assumption that the implementations would be very similar, I would suggest having one function so for the sake of DRY. It is however also legitimate to use wrapper functions around this function to make your code more instantly readable

Answer 3

This depends on the context. Both options have its pros and cons and tradeoffs are to be made.

Both at once

Freedom of choice - allowing setChannelOneVoltage(220) as well as setVoltage(1, 220).

Everybody's happy!

But it invites inconsistency. One programmer will be using the former, perhaps unaware of the shortcut, another will prefer the latter.

Third programmer comes over, searches for all usages of setChannelOneVoltage (for some reason) and it may not occur to him that he's overlooking setVoltage(1... calls.

Is it a dealbreaker? No, but it should be taken into account. Every optional shortcut will be used inconsistently and therefore add a bit of noise.

A pet peeve of mine is the coexistence of collection.size() == 0 and collection.isEmpty(), for example. It's noisy. But since both options are equally valid, enforcing a consistent convention is hard, as it boils down to one's taste ultimately.

Shortcut methods only

Now we are hiding the setVoltage(x implementation and only make it accessible by our sugar shortcuts.

But what if you need to quickly set voltage on all channels? Instead of a simple loop, you are now writing:

setChannelOneVoltage(voltage)
setChannelTwoVoltage(voltage)
setChannelThreeVoltage(voltage)
setChannelFourVoltage(voltage)

Okay, we can enwrap this into another shortcut function: setAllChannelsVoltage.

But what if we need to set all channels apart from number 1? Do we follow up with another helper method: setAllChannelsButTheFirst...

Depending on the nature of the task, it may be hard to maintain.

Only the raw, parameterized method

It is now hard to find all usages of setVoltage(1.... You need to resort to text search, and it will fail in case of i = 1; setVoltage(i, 220) or some other of all the endless possibilities.

It makes code more abstract and adds to the number of parameters - if it's a choice between one or two, that's not a big deal, but if the number of parameters has grown beyond that already, you should think twice every time before adding just one more.

If there is only ever 4 channels and no more, setVoltage(int channel puts the code at risk of out of range exceptions, which you are protected from by design if you only provide shortcuts.

Code that contains setVoltage(5 will compile and crash in runtime, but it won't build at all if it contains a call to setChannelFiveVoltage as the method doesn't exist, making the error much easier to catch out.

To sum up

Chosing one of these design approaches requires us to answer certain questions.

Is setting voltage on channel 2 or 3 a special case in its own right so that we might want to identify it in code and easily distinguish from all the others?

Is it likely that we will need to iterate over these channels and set their voltage conditionally?

Or that we would be setting the voltage of a channel whose ordinal number is computed or derived from some other data?

Has the parameterized version been already in use for some time? In such case the resulting inconsistency would strike us harder, because preexisting code won't be using the shortcuts and other programmers won't be used to them either, and stick to what they know.

In other words - which of the pros and cons that I listed above applies in your situation and to what extent? Some may be relevant, while others not so much. Calculate the tradeoffs for the task at hand, there is no silver bullet here; YMMV.

Answer 4

If you are using an object oriented language, could it make sense to make the channel an object?

channels = new Channel[4];
channels[0] = new Channel();
...
channels[0].SetVoltage(220);

Answer 5

You could use an enumeration for your channel:

enum Channel
{
    C1,
    C2,
    C3,
    C4
};

then you can use a single method:

setVoltage(Channel.C1,voltage);
setVoltage(Channel.C2,voltage);
setVoltage(Channel.C3,voltage);
setVoltage(Channel.C4,voltage);

and you can use it in a loop:

for(Channel channel = Channel.C1; channel <= channel.C4; channel++)
{
    setVoltage(channel,voltage);
}

Answer 6

Both.

Use wrapper functions with few parameters to create intuitivity, call a single function with many parameters inside the wrappers to provide compact, coherent code.

Answer 7

The question is twofold. For the current example and the given set of circumstances, use approach 2. In a more general sense the answer would be "it depends" and there are more approaches than just two.

Some more variants on the topic:

Approach 3

setVoltage( channel, voltage)
setMaxCurrent( channel, maxCurrent)
setMaxWorkingTime( channel, maxHours)
setChannelSettings( channel, voltage, maxCurrent = null, maxHours = null) 

That one is good if only a few args come into play. It quickly becomes ugly with more than 3 or 4 args.

Approach 4

struct settings {
  int channel
  nullable double voltage
  nullable double maxCurrent
  nullable double maxHours
  // add new fields here
}

setVoltage( List<settings> settings)
setVoltage( settings)

Easily extendable, can be made forward-compatible, but requires the caller to set up a rather complex structure for each call. The list<> version enables to set all values with one call, which may be important if the call is expensive, e.g. if it is an RPC with high latencies.

... and so on. As you see, all approaches have their own advantages and disadvantages, and the question Which option is better? should always be completed with Which option is better for that particular purpose, use case and set of restrictions that we face?.

Answer 8

Assuming that all channels are same, setChannelOneVoltage(voltage) and setChannelTwoVoltage(voltage) are doing the same, but on different channels. If you now can identify those channels easily by a function that takes the channel number as the argument, the second approach, using setVoltage(channelNo,voltage) would be the better one, mainly because of DRY and flexibility.

If identifying the channels is more complex (and perhaps different for each channel), you should consider to wrap each channel into an object. But then again, the second approach would be the better one, now with setVoltage(channelObj,voltage).

But if setting the voltage is significantly different for each channel, the first approach would be better; having a distinct function/method for each channel is better than a construct like

setVoltage( channel, voltage )
{
   switch( channel )
   {
      case 1: // do it for channel 1
        // Lengthy, complex code going here ...
      case 2: // do it for channel 2
        // Lengthy, complex code going here ...
      ...
   }
}

But for convenience, you can offer a setVoltage(channel,voltage) method even in this scenario; it calls the setChannel#Voltage(voltage) methods in a switch - or using reflection, if your programming language do provide such a feature - in C, you can implement something like that as a macro instead.

An alternative approach for an object oriented language would be to define a Channel interface class and setVoltage(voltage) as one of the methods. This would address DRY and flexibility as well as all the distinctions that the different channels may have in retrieval and/or setting the voltage.

Answer 9

If you're using c#, make an extension method. that would allow you to do this: var channel1 = new channel(...); channel1.SetVoltage(15);

var channel2 = new channel();
channel2.SetVoltage(56);

some more info on extension methods: http://msdn.microsoft.com/en-us/library/bb383977.aspx