6

Lets say I have an extremely robust and versatile function:
void DoAnything(action, target, context)

In my program, 90% of the time I call this function, it's with the same parameters:
DoAnything(eat, food, lunchtime);

Is it bad form for me to define a function such as:
void EatLunch(){ DoAnything(eat, food, lunchtime); }

And call that instead?

  • I would say that pretty much all functions do this in one way or another. – Kaz Dragon Oct 7 '11 at 10:03
  • 2
    It would be bad practice not to do this. – Doc Brown Oct 7 '11 at 13:01
8

It's a good thing to do when you find the code is more readable and more maintainable when wrapped using the invariant arguments. I do it often.

| improve this answer | |
6

When you're writing a library, adding convenience wrappers is often a service to your users.

For example, suppose I've written a stream compression library in C. My highly versatile entry point might be:

Stream *compress_stream(Stream *input, StreamOptions *opts);

However, many users will simply want a function like this:

void *compress(const void *data, size_t length, size_t *out_length);

compress_stream is more versatile: it can both consume input and produce output lazily, and the user can supply additional parameters (e.g. algorithm, quality, etc.)

However if a user simply has a blob of data they want to make smaller, my compress_stream function will be extremely cumbersome by itself. The user will have to learn about my Stream object:

typedef struct Stream Stream;

struct Stream
{
    size_t (*read)(Stream *s, void *buffer, size_t len);
    void free(Stream *s);
};

Then they'll have to implement compress in their own terms:

typedef struct
{
    Stream stream;
    const char *data;
    size_t remaining;
} SimpleStream;

size_t SimpleStream_read(Stream *s, void *buffer, size_t len)
{
    SimpleStream *ss = container_of(s, Stream, stream);

    if (len > ss->remaining)
        len = ss->remaining;

    memcpy(buffer, ss->data, len);
    ss->data += len;
    ss->remaining -= len;
    return len;
}

void SimpleStream_free(Stream *s)
{
    free(container_of(s, Stream, stream));
}

Stream *SimpleStream_new(const void *data, size_t length)
{
    SimpleStream *ss = stream_new(sizeof(SimpleStream),
                                  SimpleStream_read,
                                  SimpleStream_free);
    if (ss == NULL)
        return NULL;

    ss->data = data;
    ss->remaining = length;
    return &ss->stream;
}

void *consume(Stream *s, size_t *out_length)
{
    char *buffer;
    size_t buffer_length = 0;
    size_t buffer_alloc = 16;

    buffer = malloc(buffer_alloc + 1);
    if (buffer == NULL)
        return NULL;

    for (;;) {
        size_t readlen;

        readlen = s->read(s, buffer + buffer_length, buffer_alloc - buffer_length);
        if (readlen == 0)
            break;

        buffer_length += readlen;
        assert(buffer_length <= buffer_alloc);
        if (buffer_alloc - buffer_length < 16) {
            char *tmp;

            buffer_alloc *= 2;
            tmp = realloc(buffer, buffer_alloc + 1);
            if (tmp == NULL) {
                free(buffer);
                return NULL;
            }
            buffer = tmp;
        }
    }

    buffer[buffer_length] = 0;
    *out_length = buffer_length;
    return buffer;
}

void *compress(const void *data, size_t length, size_t *out_length)
{
    Stream *input;
    Stream *output;
    void *ret;

    input = SimpleStream_new(data, length);
    output = compress_stream(input, NULL);
    input->free(input);
    ret = consume(output, out_length);
    output->free(output);
    return ret;
}

This may be a contrived example, but I've seen libraries neglect to include simple wrappers on multiple occasions. For users to implement those wrappers in their code, they will have to learn more about your library than they really wanted to.

| improve this answer | |
5

Nope, it's good form to do this. By defining what it means to EatLunch() in one place you reduce code repetition. Also, if ever you need to change what it means to EatLunch() then you only have to do so in one place.

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.