58

In a function that takes several arguments of the same type, how can we guarantee that the caller doesn't mess up the ordering?

For example

void allocate_things(int num_buffers, int pages_per_buffer, int default_value ...

and later

// uhmm.. lets see which was which uhh..
allocate_things(40,22,80,...
πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
Anonymous Entity
  • 3,254
  • 3
  • 27
  • 41
  • 4
    Compiler can help you most of the times. Otherwise, it's your(programmer) responsibilities. – P.P Aug 08 '16 at 19:05
  • 7
    Isn't this easy in C++ by using specific types? – alk Aug 08 '16 at 19:15
  • If the parameters are organized logically (a,b.c,d,e...) it becomes easy to remember all of them in correct order. – Raktim Biswas Aug 08 '16 at 19:21
  • 18
    Could you use [method chaining](https://en.m.wikipedia.org/wiki/Method_chaining)? Something like `allocate_thing().buffers(40).pages_per_buffer(22).default_value(80)` – Mark Plotnick Aug 08 '16 at 19:42
  • 6
    This is a good question. I think the only real solution is to create value-types for each item that needs configuring. Like the `` library uses *durations* such as `std::chrono::seconds` to configure time periods. – Galik Aug 08 '16 at 20:01
  • 3
    This example is particularly bad because exchanging the values for num_buffers and pages_per_buffer might actually work, but with hugely reduced performance (10000 buffers with one page per buffer might be highly inefficient compared to one buffer with 10,000 pages). In other cases, you would often get a highly visible bug, but here you possibly wouldn't. – gnasher729 Aug 09 '16 at 14:04
  • 6
    @gnasher - agreed, that's a dangerous function - that makes it a particularly *good* example. – Toby Speight Aug 09 '16 at 14:23
  • 3
    I suggested adding explicit parameter naming to function calls years (20+) ago, along the lines of `alloc(bufs = 40, pages = 22, dfl = 80)`, but most people complained that this was not "object-oriented" enough. So while the method name and parameter types (reasonably) comprise elements of the signature, some people refuse to consider that the parameter names could likewise be part of the signature. – David R Tribble Aug 09 '16 at 15:06
  • 1
    You could perhaps use the [Named Parameter Idiom](http://stackoverflow.com/a/2700976/10077). – Fred Larson Aug 09 '16 at 15:07
  • @P.P.: I can't see how the compiler could possibly help the programmer if he swaps the ordering of multiple arguments of the same type in a function call. How can it possibly know which arguments go with which parameters if they all have the same type? – David R Tribble Aug 09 '16 at 15:14
  • Why would the caller mess up the orderings? You can either declare enumerations, define's ,use a `struct`, or lay the prototype in front of you and pass values accordingly. – machine_1 Aug 09 '16 at 19:52
  • how is the compiler allowed to reorder parameters? – phuclv Aug 10 '16 at 02:21
  • 1
    The oldest trick in the book: `allocate_things(/* buffers = */ 40, /* pages_per_buffer = */ 22, /* default_value = */ 80,...` – higuaro Aug 10 '16 at 08:11

7 Answers7

68

A typical solution is to put the parameters in a structure, with named fields.

AllocateParams p;
p.num_buffers = 1;
p.pages_per_buffer = 10;
p.default_value = 93;
allocate_things(p);

You don't have to use fields, of course. You can use member functions or whatever you like.

Dietrich Epp
  • 205,541
  • 37
  • 345
  • 415
  • You could do this, but still `allocate_things` is a bad name and `AllocateParams` is not better. Creating structs of more or less unrelated variables is likely to obscure things even more. – Frank Puffer Aug 08 '16 at 19:14
  • 10
    @FrankPuffer: Yes, agreed, but this is not the code review stack exchange. If you have comments about the original author's code, they belong as comments on the question, not on the answers. This code sample is designed to illustrate a particular technique and nothing else. – Dietrich Epp Aug 08 '16 at 19:24
  • 11
    @FrankPuffer: I think it's clear that these are just placeholder names. – Dietrich Epp Aug 08 '16 at 19:26
  • 2
    What I wanted to point out is that your answer does provide a workaround that resolves the issue but that the actual problem lies somewhere else: Fuctions with three or more arguments of the same type simply should be avoided. Not only because the interface gets difficult to use correctly but also because it is very likely that the function does too many things and should be split up. – Frank Puffer Aug 08 '16 at 19:35
  • 3
    I'm not completely convinced. Doesn't this just move the problem elsewhere? Can't you accidentally set the member variables just as wrongly as parameter variables? – Galik Aug 08 '16 at 19:47
  • 6
    @Galik With this pattern, the programmer would have to be a lot more asleep to get it wrong, since they are required to refer to the fields by name. (until they forget why they did it and think it's clever to pass by braced init-list, ending with the original problem + new pointless filler [edit: nate, we've done it again]) – underscore_d Aug 08 '16 at 19:51
  • 4
    @Galik i.e. `allocate_things({ 1, 10, 93 });` – nate Aug 08 '16 at 19:51
  • 9
    @FrankPuffer: I think it's clear that this is not supposed to be a real function. Your assertion that the function "does too many things" is basically unfounded—the only information you have for that is the function name, which again, is obviously made up! It might as well be `foo()`. This kind of tangential commentary is my biggest source of frustration with Stack Overflow. – Dietrich Epp Aug 08 '16 at 20:28
  • 4
    @FrankPuffer: If you think that there's a real, bona fide issue not being addressed, go put that comment on the question or write your own answer if it is substantial enough. The issue of a function doing "too many things" has absolutely nothing to do with the actual question being asked. – Dietrich Epp Aug 08 '16 at 20:30
  • 1
    @DietrichEpp: I don't exactly understand what you mean by "This kind of tangential commentary" but I really didn't intend to make you frustrated. I wrote my original comment after having written my own answer to this question. While your answer is good and surely correct, I still find it likely that in the long run my suggestion might be more helpful for the OP. Of course you are right that there is some speculation in it, that's why I wrote "likely", but it is based on my experience with similar cases. – Frank Puffer Aug 08 '16 at 21:03
  • 1
    @DietrichEpp: Just noticed that someone voted to close because this question attracts opinionated answers - seems to be correct :) – Frank Puffer Aug 08 '16 at 21:06
  • 2
    If you do use a structure, you'll probably find people using the implicit initialization syntax `allocate_things({1, 10, 93})`, putting you right back where you started, but now with an extra pair of braces for visual noise. You could prevent that by specifying a constructor, but you're still making life harder for your users. – Toby Speight Aug 09 '16 at 14:26
  • Yes, there are some interesting tradeoffs here. Sometimes there's a fine line between making an easy-to use API and treating your users (who are competent programmers, hopefully) like children. – Dietrich Epp Aug 09 '16 at 14:32
30

Two good answers so far, one more: another approach would be to try leverage the type system wherever possible, and to create strong typedefs. For instance, using boost strong typedef (http://www.boost.org/doc/libs/1_61_0/libs/serialization/doc/strong_typedef.html).

BOOST_STRONG_TYPEDEF(int , num_buffers);
BOOST_STRONG_TYPEDEF(int , num_pages);

void func(num_buffers b, num_pages p);

Calling func with arguments in the wrong order would now be a compile error.

A couple of notes on this. First, boost's strong typedef is rather dated in its approach; you can do much nicer things with variadic CRTP and avoid macros completely. Second, obviously this introduces some overhead as you often have to explicitly convert. So generally you don't want to overuse it. It's really nice for things that come up over and over again in your library. Not so good for things that come up as a one off. So for instance, if you are writing a GPS library, you should have a strong double typedef for distances in metres, a strong int64 typedef for time past epoch in nanoseconds, and so on.

KindDragon
  • 6,558
  • 4
  • 47
  • 75
Nir Friedman
  • 17,108
  • 2
  • 44
  • 72
  • 2
    For integers in particular, scoped enum is a decent choice. – T.C. Aug 08 '16 at 19:47
  • You can go one step further with this approach by using user defined literals to reduce the syntactic overhead of using customs types when making calls. – nate Aug 08 '16 at 19:50
  • 2
    you can get a call that looks like `allocate_things(40_buffers,22_pages, 80...` and if you don't put the values in the right places, it gives you a compiler error. – nate Aug 08 '16 at 19:57
30

If you have a C++11 compiler, you could use user-defined literals in combination with user-defined types. Here is a naive approach:

struct num_buffers_t {
    constexpr num_buffers_t(int n) : n(n) {}  // constexpr constructor requires C++14
    int n;
};

struct pages_per_buffer_t {
    constexpr pages_per_buffer_t(int n) : n(n) {}
    int n;
};

constexpr num_buffers_t operator"" _buffers(unsigned long long int n) {
    return num_buffers_t(n);
}

constexpr pages_per_buffer_t operator"" _pages_per_buffer(unsigned long long int n) {
    return pages_per_buffer_t(n);
}

void allocate_things(num_buffers_t num_buffers, pages_per_buffer_t pages_per_buffer) {
    // do stuff...
}

template <typename S, typename T>
void allocate_things(S, T) = delete; // forbid calling with other types, eg. integer literals

int main() {
    // now we see which is which ...
    allocate_things(40_buffers, 22_pages_per_buffer);

    // the following does not compile (see the 'deleted' function):
    // allocate_things(40, 22);
    // allocate_things(40, 22_pages_per_buffer);
    // allocate_things(22_pages_per_buffer, 40_buffers);
}
sergej
  • 17,147
  • 6
  • 52
  • 89
  • 4
    ... *oh wow* . +1; this is **very** interesting. But I don't know whether I do or don't want to find a scenario where I would need it... ;-) – underscore_d Aug 09 '16 at 13:25
  • 1
    This looks like it can be macro-ified. – rubenvb Aug 09 '16 at 15:38
  • What if 40 were a variable instead of a literal? – Barry Aug 09 '16 at 20:25
  • @Barry I guess if 40 were a variable, it would have a meaningful name. `operator""` would not be used. – sergej Aug 09 '16 at 20:34
  • Are user-literals with names starting with `_` allowed? I recall that for most things that have names, C++ usually reserves the ones with names that start from `_`. – Joker_vD Aug 09 '16 at 23:02
  • 3
    @Joker_vD: User-defined literal suffixes are the other way around. Suffixes that _don't_ start with `_` are reserved. (C++11 §17.6.4.3.5; don't have the section for later versions.) – cHao Aug 10 '16 at 06:19
9

(Note: post was originally tagged 'C`)

C99 onwards allows an extension to @Dietrich Epp idea: compound literal

struct things {
  int num_buffers;
  int pages_per_buffer;
  int default_value 
};
allocate_things(struct things);

// Use a compound literal
allocate_things((struct things){.default_value=80, .num_buffers=40, .pages_per_buffer=22});

Could even pass the address of the structure.

allocate_things(struct things *);

// Use a compound literal
allocate_things(&((struct things){.default_value=80,.num_buffers=40,.pages_per_buffer=22}));
Community
  • 1
  • 1
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • 1
    But this is about C++. Which doesn't import compound literals from C. – underscore_d Aug 08 '16 at 19:39
  • 1
    @underscore_d The post _was_ about C until edited. (The post does still makes sense in a C context - unclear on OP/πάντα ῥεῖ change. - now see it to correlate to title) – chux - Reinstate Monica Aug 08 '16 at 19:40
  • 1
    Yup, just saw that. Fair play as per the original tags. Although the title always disagreed. If only people would tag what they really mean... sigh – underscore_d Aug 08 '16 at 19:40
  • @underscore_d: I think the problem is that people mean the wrong things, which is harder to solve. – Dietrich Epp Aug 08 '16 at 20:34
  • 1
    Don't use a pointer, use a reference. Using a pointer means that the function has to handle the `nullptr` case, and using a reference requires the object to exist. Also nowadays the general advice is to avoid pointers and use smart pointers instead – Pharap Aug 09 '16 at 00:24
  • 1
    @Pharap Post was originally tagged C and this answer relates to that and so the reference idea of yours has merit with C++. OP's post has since dropped the `C` tag. – chux - Reinstate Monica Aug 09 '16 at 01:08
7

You can't. That's why it is recommended to have as few function arguments as possible.

In your example you could have separate functions like set_num_buffers(int num_buffers), set_pages_per_buffer(int pages_per_buffer) etc.

You probably have noticed yourself that allocate_things is not a good name because it doesn't express what the function is actually doing. Especially I would not expect it to set a default value.

Frank Puffer
  • 8,135
  • 2
  • 20
  • 45
  • 3
    And to separate responsibilities. – Charles D Pantoga Aug 08 '16 at 19:04
  • 3
    and don't use magic numbers, hard-coding parameters like you do usually leads to more pain than its worth. – Cody Aug 08 '16 at 19:06
  • lmao, why is this being downvoted? I thought it was considered good to express the possibility that people are asking X/Y questions? It's pretty valid to present alternatives that have benefits of their own. – underscore_d Aug 08 '16 at 19:42
  • 2
    this introduces unnecessary (potentially global) state to the system – nate Aug 08 '16 at 19:42
  • 1
    @nate Does a function count as "state"? I must've missed that memo. Or do you mean that having separate functions for properties that might have to interact later means that they need to be stored during the process of setting them up? – underscore_d Aug 08 '16 at 19:43
  • 4
    in order to for `set_XXX` to affect a future `allocate_things` call, the arguments must be stored somewhere. – nate Aug 08 '16 at 19:44
  • @nate gotcha, realised that soon after and edited it in :) yup, that might well be true for some contexts. still, if the properties are genuinely separate (as isn't clear in this case), then separation of concerns is a good thing. as always, there's no single good answer for all situations. – underscore_d Aug 08 '16 at 19:45
  • @underscore_d I agree with separation of concerns to some extent, but the question is about protecting what is passed as arguments to function, this is where the type system shines if used correctly. – nate Aug 08 '16 at 19:48
  • 1
    @nate Yeah, strong typedefs would be preferable in the case quoted here, but sadly that idea seems highly resistant to standardisation. Still, the breadth of C++ means there are ways to roll our own, e.g. http://programmers.stackexchange.com/questions/243154/c-strongly-typed-typedef – underscore_d Aug 08 '16 at 19:54
7

Just for completeness, you could use named arguments, when your call becomes.

void allocate_things(num_buffers=20, pages_per_buffer=40, default_value=20);
// or equivalently
void allocate_things(pages_per_buffer=40, default_value=20, num_buffers=20);

However, with the current C++ this requires quite a bit of code to be implemented (in the header file declaring allocate_things(), which must also declare appropriate external objects num_buffers etc providing operator= which return a unique suitable object).

---------- working example (for sergej)

#include <iostream>

struct a_t { int x=0; a_t(int i): x(i){} };
struct b_t { int x=0; b_t(int i): x(i){} };
struct c_t { int x=0; c_t(int i): x(i){} };

// implement using all possible permutations of the arguments.
// for many more argumentes better use a varidadic template.
void func(a_t a, b_t b, c_t c)
{ std::cout<<"a="<<a.x<<" b="<<b.x<<" c="<<c.x<<std::endl; }
inline void func(b_t b, c_t c, a_t a) { func(a,b,c); }
inline void func(c_t c, a_t a, b_t b) { func(a,b,c); }
inline void func(a_t a, c_t c, b_t b) { func(a,b,c); }
inline void func(c_t c, b_t b, a_t a) { func(a,b,c); }
inline void func(b_t b, a_t a, c_t c) { func(a,b,c); }

struct make_a { a_t operator=(int i) { return {i}; } } a;
struct make_b { b_t operator=(int i) { return {i}; } } b;
struct make_c { c_t operator=(int i) { return {i}; } } c;

int main()
{
  func(b=2, c=10, a=42);
}
Walter
  • 44,150
  • 20
  • 113
  • 196
6

Are you really going to try to QA all the combinations of arbitrary integers? And throw in all the checks for negative/zero values etc?

Just create two enum types for minimum, medium and maximum number of buffers, and small medium and large buffer sizes. Then let the compiler do the work and let your QA folks take an afternoon off:

allocate_things(MINIMUM_BUFFER_CONFIGURATION, LARGE_BUFFER_SIZE, 42);

Then you only have to test a limited number of combinations and you'll have 100% coverage. The people working on your code 5 years from now will only need to know what they want to achieve and not have to guess the numbers they might need or which values have actually been tested in the field.

It does make the code slightly harder to extend, but it sounds like the parameters are for low-level performance tuning, so twiddling the values should not be perceived as cheap/trivial/not needing thorough testing. A code review of a change from allocate_something(25, 25, 25);

...to

allocate_something(30, 80, 42);

...will likely get just a shrug/blown off, but a code review of a new enum value EXTRA_LARGE_BUFFERS will likely trigger all the right discussions about memory use, documentation, performance testing etc.

Dan Haynes
  • 388
  • 3
  • 6