Skip to main content
5 of 7
deleted 17 characters in body
Jerry Coffin
  • 33.6k
  • 4
  • 75
  • 143

@vnp's code is solid and helpful, but his stringify_state_helper is a single-purpose function, and still leaves a degree of repetition and memory management in stringify_state. I'd rather have general-purpose to_string that takes printf-style arguments, allocates sufficient space for the converted result, and prints into that space, and returns the result:

#include <stdio.h>
#include <stdarg.h>
#include <stdlib.h>

char *to_string(char const *fmt, ...) { 
    va_list args;
    va_start(args, fmt);

    va_list args_dupe;
    va_copy(args_dupe, args);

    int size = vsnprintf(NULL, 0, fmt, args);

    char *ret = malloc(size+1);

    if (ret != NULL)
        vsnprintf(ret, size+1, fmt, args_dupe);

    va_end(args);
    va_end(args_dupe);

    return ret;
}

In fairness, this does require a little more code, and the v*printf functions (and argument-list macros) are a bit less known, so some may find it a bit more difficult to understand.

On the other hand, in exchange for that bit of extra investment, we get something that's more general, and works much more as I think most people would expect--for example, something like this:

char *s = to_string("%d, %d", 1, 2);

Using this, stringify_state should turn out something like this:

char* stringify_state(State* state) {
    Pet* p = state->pet;
    Settings* s = state->settings;

    return to_string("%lld %f %f %f %f %f %f %f %f %f %f",
                     state->last_update_time,
                     p->health, 
                     p->max_health, 
                     p->satiation, 
                     p->max_satiation,
                     s->milliseconds_per_tick, 
                     s->hunger_pain_per_tick, 
                     s->hunger_per_tick,
                     s->satiated_heal_per_tick,
                     s->pain_per_wrong_answer,
                     s->satiation_per_right_answer);
}

#Levels of abstraction

It may just be a pet peeve of mine, but I think this divides into a cleaner set of layers of abstraction. With your original code, we have a single function that mixes a fairly high level of abstraction (format a State into a string) with lower levels of abstraction (e.g., memory management).

@vnp's code sort of inverts the layers of abstraction, so we have the higher level of abstraction at the bottom, and the lower level of abstraction above it.

This one gets the layers of abstraction closer to how I'd like to see them: the memory management and general purpose stringifying is at the bottom of the stack, and the more specific higher level concern of Stringifying a State is at an upper layer. So, something like a call graph accurately reflects the levels of abstraction being dealt with in the code.

#Other Points

  1. Given the number (and length) of arguments you're passing, I'd prefer to see the arguments passed one per line as I've formatted them above, rather than a variable number per line to justify out to some particular right margin.

  2. Since stringify_state isn't intended to modify the state whose address is being passed, it's probably better to define the parameter as a State const *. Likewise with the local variables, so the first lines would probably be better written something like this:

     char* stringify_state(State const* state) {
         Pet const* p = state->pet;
         Settings const* s = state->settings;        
    
Jerry Coffin
  • 33.6k
  • 4
  • 75
  • 143