tags:

views:

156

answers:

6

When designing a C API for configuring a library/utility, I have a co-worker who prefers to lump all configuration parameters into 1 function call. For example:

int set_diagnostic_email_config( char *to_address,
                                 bool include_timestamp,
                                 bool send_for_crashes,
                                 bool send_daily_status,
                                 bool send_on_event1,
                                 bool send_on_event2 )

A similar "get" function exists with many parameters.. The main advantage of this method is that if someone adds a new option, e.g., "bool send_on_event3", then because the prototype changed you are forced to update every place this function call is used (assuming there are multiple places where people call this).

I prefer something along the lines of:

int get_diagnostic_email_config( struct email_config *p_config );
int set_diagnostic_email_config( struct email_config *p_config );

where you just change the structure elements as needed. But... if someone updates the email_config" structure it doesn't force people to update all the places where it is used (even though we often want to..). Plus my co-worker complains that if someone just tries to initialize "email_config" by hand, then if things get added later then those new fields will be uninitialized with no warnings.

Is there any strong consensus on which method is preferred? or perhaps there is another alternative I'm missing?

A: 

How is having to update the function call in all places a good thing? Of course, you should rebuild everything with the new updated headers, but ideally you'll want to have to make as few changes in the code as possible every time the config is changed.

Tal Pressman
A: 

The "numerous parameters" approarch has a big disadvantadge that you need to alter a lot of code each time a new parameter is introduces just for the sake of passing that parameter down the call stack. With a structure you only need to change the places that actually use the parameters. The need to often modify tons of code can cause error planting on its own and that would outweight the advantages of compile-time checks of all parameters being present.

sharptooth
+11  A: 

A struct is better than a long list. Long list is hard to maintain, as nobody remembers the exact order.

You can create a constructor that fills this struct with safe (default and/or invalid) entries. Always (probably in the accessor function) checking for invalid values will make it easy to spot errors in initialization.

You can hide a magic number in this struct. The first field is CONFIG_MAGIC that needs to be equal to a constant that you defined. You set this field in the constructor and expect to be set at all times. This you avoid somebody just malloc()ing the struct and initalizing it by hand. Such programmer would need to learn about this CONFIG_MAGIC constant and is more than likely to find and use the proper constructor.

Tadeusz A. Kadłubowski
+1. Excellent advice.
DevSolar
+1 basically it is OOP - easier to manage changes, keep implementation with the state - constructor/destructor to initialize cleanup
stefanB
Thanks! I especially like the CONFIG_MAGIC idea.
Will
+3  A: 

I would use the following way which is an extension to your way.

struct INFO
{
    char *to_address;
    bool include_timestamp;
    bool send_for_crashes;
    bool send_daily_status;
    bool send_on_event1;
    bool send_on_event2;
};

struct INFO *createINFO()
{
    // initialize to defaults.

    // return a pointer to the new created struct.
}

void include_timestamp(struct INFO *info, bool vInclude_timestamp)
{
    // set the field.
}

// add the required setters...

void destroyINFO(struct INFO *info)
{
    // destroy the struct.
}

This way, you can add 'setters' on demand, when ever you add a new field. Without allowing the user to mess with the struct itself.

AraK
I would rather not do these memory management tasks.
J-16 SDiZ
well, you are correct :)
AraK
A: 

I would use

int set_diagnostic_email_config( char *to_address,
                                 bool include_timestamp,
                                 int event_type ) {
    switch (event_type) {
        case 1:
        ...
    }
}

or

int set_diagnostic_email_config( char *to_address,
                                 bool include_timestamp,
                                 int event_type, void *details )

Having a event_type as an integer means you can add new event without changing the signature. An extra void* give a optional structure for each event type.

If you use struct, changes to it may not be always binary compatible.

J-16 SDiZ
+1  A: 

Long parameter lists are not readable, in particular if it is a list of bools. Everytime you come to a function call that looks like this:


    set_diagnostic_email_config("[email protected]", 0, 1, 0, 1, 0, 1, 0, 0, 0);

you have to look into the documentation to see what this digit pattern is good for. And if you are using the function, you want to use in most cases with some sane defaults, ie. you end up in copying this line from some where else.

If you have only boolean values, I'd use flags, which you can combine with ORing. Here is a possible example:


   typedef enum {
       FLAG_DEFAULT = 0,
       FLAG_INCLUDE_TIMESTAMP = 0x1,
       FLAG_SEND_FOR_CRASHES = 0x2,
       FLAG_SEND_DAILY_STATUS = 0x4,
       FLAG_SEND_ON_EVENT1 = 0x8
    } Email_Flags;

    int set_diagnostic_email_config(const char *address, unsigned int flags);

Now you can call the function like this:

    set_diagnostic_email_config("[email protected]", FLAG_SEND_DAILY_STATUS | FLAG_SEND_ON_EVENT1);

This code is easy to read, you don't need to know every option to understand it. And this code is easy to write, because the order of the "paramters" (actually the order of the flags) is not important. And this function is easy to extend, you can simply add more flags, let's say FLAG_SEND_ON_EVENT2 and you don't need to change any function call, as long as you want to change its behavior.

quinmars