48

I've got a function which looks like this:

bool generate_script (bool net, bool tv, bool phone,
                        std::string clientsID,
                        std::string password,
                        int index, std::string number, 
                        std::string Iport, std::string sernoID,
                        std::string VoiP_number, std::string  VoiP_pass,
                        std::string target, int slot, int port, 
                        int onu, int extra, std::string IP, std::string MAC);

In my opinion it looks ugly. What is the proper way of handling this problem? Should I create few vectors with different data types (int, string and bool) and pass them as arguments to this function?

Braiam
  • 1
  • 11
  • 47
  • 78
Tomasz Kasperczyk
  • 1,991
  • 3
  • 22
  • 43
  • 6
    I would discourage using vector, a struct would be a far more appropriate solution. – ctor Jul 25 '14 at 09:19
  • there is also boost named parameters, if you have lots of useful defaults – PlasmaHH Jul 25 '14 at 21:24
  • 1
    Group them into one or more `struct`s. Also, don't forget to use `const` reference type for strings in case you don't modify it in function, that increases speed. – ST3 Jul 28 '14 at 13:49
  • 2
    And if that function sole purpose becomes handling a struct, why not turn it into a method of that struct? – didierc Aug 01 '14 at 15:49
  • Something no one seems to mentioned is that they're all passed by value :( Passing by reference would avoid so many allocations – RedOrav Jul 11 '21 at 16:45

6 Answers6

85

If all these parameters are meaningfully related, pack them in a structure.

Quentin
  • 62,093
  • 7
  • 131
  • 191
  • Thank you, I will do that. It's a good solution to this particular problem, but what if those arguments were not meaningfully related? – Tomasz Kasperczyk Jul 25 '14 at 09:22
  • 3
    Some will be, so make two or three. Or make a specific parameters object for this method (they are related because they are params to this method). – Wilbert Jul 25 '14 at 09:23
  • 26
    If your arguments are not meaningfully related your function might try to serve multiple purposes at once. – ogni42 Jul 25 '14 at 09:26
  • 14
    Note that having many, many unrealated parameters leads to the question of whether your function is doing too much stuff. – Quentin Jul 25 '14 at 09:27
  • Actually it's doing only one thing - it's generating a TCL script which automatically configures a network device. These parameters are related so I can pack them into a struct. I was just suggesting that one might encounter a similar problem but with totally unrelated parameters. – Tomasz Kasperczyk Jul 25 '14 at 09:33
  • @user3125731 yep, I was referring to said hypothetical issue :) – Quentin Jul 25 '14 at 09:34
  • I am the only one here that doens't like to use struct in C++? – dynamic Jul 25 '14 at 16:59
  • 6
    @dynamic Possibly. `struct`s are nice for when you really just want a collection of public fields, as here. To me, `class` is an indicator that there's probably private state being managed. `struct` on the other hand connotes flat, plain data, which is exactly what is needed here. – Stuart Olsen Jul 25 '14 at 18:11
  • 2
    Is it also possible that `script` should be an object, each of your arguments should be properties, and that `generate` should be a method in the class? – sudo make install Jul 26 '14 at 02:21
45

Put them in a struct

Create a structure 

struct GenerateScriptParams { /* ... */ };

and put all the parameters in there. You can actually provide default values for the initialization of the struct as well by implementing a default constructor or, in C++11, by providing default initialization of individual members. You can then change the values that are not supposed to be defaulted. This selective picking of non-default parameters is not possible for a function call with lots of parameters in C++.

Making the interface nice for the caller

Yet, the usage is a little ugly, since you have to create a temporary name object, then change the values that should not be default and then pass the object to the function:

GenerateScriptParams gsp;
gsp.net = true;
gsp.phone = false;
gps.extra = 10;
generate_script( gsp );

If you call that function in several different places, it makes sense to avoid this uglyness by providing mutating member functions that can be chained:

GenerateScriptParams & GenerateScriptParams::setNet  ( bool val );
GenerateScriptParams & GenerateScriptParams::setTV   ( bool val );
GenerateScriptParams & GenerateScriptParams::setPhone( bool val );
// ... //

Then calling code can write 

generate_script( GenerateScriptParams()
    .setNet(true),
    .setPhone(false),
    .setExtra(10) );

without the above uglyness. This avoids the named object that is only used once.

Ralph Tandetzky
  • 511
  • 3
  • 3
  • 34
    Because I haven't done any C++ for a while I initially read `GenerateScriptParams & GenerateScriptParams::setNet` as being a bit-wise `AND` between two instances of `GenerateScriptParams` - which confused the hell out of me. Personally I prefer `GenerateScriptParams& GenerateScriptParams::setNet`, as the ampersand is bound to the class name when considering the function's return type. – Peter M Jul 25 '14 at 14:17
24

I personally do not believe that moving all the arguments in one struct will make the code much better. You just move dirt under the carpet. When you are going to deal with the creation of the struct you have the same problem.

The question is how much reusable this struct will be? If you end up with a 18 parameters for one function call something it is not quite right in your design. After further analysis you may discover that those parameters can be group in several different classes and those classes could be aggregated to one single object that will be the input of your function. You may want also prefer classes to struct in order to protect your data.

EDIT

I will give you a small example to describe why several classes are better than one monolithic struct. Let's start counting the tests that you need to write to cover the function above. There are 18 parameters as input (3 boolean). So we are going to need at least 15 tests only to validate the input (assuming the values are not interconnected).

The overall number of tests is impossible to be calculated without the implementation, but we can have an idea of the magnitude. Let take the lower bound all the input can be treat as boolean the number of possible combination are 2^18 so around 262000 tests.

Now, what happen if we split the input in several objects?

First of all, the code to validate the input is moved away from the function to the body of every single object (and it can be reused).

But more importantly the number of tests will collapse, let say in group of four (4,4,4 and 4 params per object) the total number of tests is only:

2^4 + 2^4 + 2^4 + 2^4 + 2^4 = 80

The fifth attributes is due to the permutation of the objects with themselves.

So, what is more cost demanding? Write thousand of tests or few more classes?

Obviously, this is a crude simplification, however, it will underlying the core of the problem. A clutter interface is not just matter of style or an inconvenient for the developer it is a true impediment to produce quality code.

This is the most important lesson I ever learnt in my career as a professional developer: "Big classes and fat interfaces are evil". That's just my heuristic version of the single responsibility principle (I have notice that the SRP can be tricky to get it right, what it seems reasonable to be single responsibility it can be not quite the same after a hour coding, so I used some heuristic rule to help me to revaulate my initial choices).

Alessandro Teruzzi
  • 3,918
  • 1
  • 27
  • 41
  • 3
    -1 Creating "several different classes" to fix a relatively simple problem is over engineering through the roof. – Paul Manta Jul 26 '14 at 10:22
  • 3
    @PaulManta I have elaborate a bit more the benefit to the "several different classes" – Alessandro Teruzzi Jul 26 '14 at 11:51
  • 4
    +1 ; this is actually the only answer that points out the real trouble with OPs code; the problem with cluttering the code with too many parameters in function call is hardly 'a relatively simple problem' - even Bloch in EJ, 2nd mentions this item as an incentive to use e.g. builder patterns and specialized classes. ; I'd give you another +1 for "Big classes and fat interfaces are evil" - I've learned exactly the same lesson (the hard way) during my OOP programming career. Modularity depends on cascading composition model, not a flat one! –  Jul 26 '14 at 12:29
14

Or you could use a fluent interface. It would look like this:

script my_script(mandatory, parameters);
my_script.net(true).tv(false).phone(true);

This is applicable if you have default values for your specified parameters or it is allowed to have a partially constructed script.

PovilasB
  • 1,488
  • 1
  • 15
  • 25
10

Ignoring the possibility or desirability of changing the function or program in some way as to reduce the number of parameters...

I have seen coding standards that specify how long parameter lists should be formatted, for cases where refactoring is not possible. One such example is using double indentations and one parameter per line (Not for all functions - only for those that have multiple-lines of parameters).

E.g.

bool generate_script (
        bool net,
        bool tv,
        bool phone,
        std::string clientsID,
        std::string password,
        int index,
        std::string number,
        std::string Iport,
        std::string sernoID,
        std::string VoiP_number,
        std::string  VoiP_pass,
        std::string target,
        int slot,
        int port,
        int onu,
        int extra,
        std::string IP,
        std::string MAC);

The point here is to create a consistent layout and look for all functions with a large number of parameters.

izb
  • 50,101
  • 39
  • 117
  • 168
  • I think that I'm gonna just pack those parameters into a struct as Quentin suggested. But thank you for explaining how such functions should be formatted. – Tomasz Kasperczyk Jul 25 '14 at 09:44
  • How many spaces should put before, e.g. `bool net,`? 4 or 8? If 4, it may be indistinguishable from the implementations in `{ ... }`. – Han Zhang Dec 13 '21 at 08:33
2

A bit late here, but since nobody has done it yet, I'd like to point out an obvious aspect of the issue: to me, a function which takes so many arguments is likely to do a lot of computation, so consider the possibility of decomposing it in smaller functions as a first step.

This should help you structuring your data.

bassfault
  • 154
  • 1
  • 7