HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Parsing key-value pairs to fill a struct

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
structvalueparsingpairsfillkey

Problem

I must read and extract some values from string. These values are coded like this:

k="11,3,1" v="140.3"


I have defined the codes and created struct with all field as well as a temp one where I store k and v. In fillFields proc I transfer values from temp struct to the right one (with the valid types). It works but I have many fields and fillFields would need to have many if-conditions. Maybe someone could give me any hint how to write it smarter.

The simplified code now:

#define ASK                       "11,3,1"
   #define BID                       "11,2,1"
   #define CLOSE                     "3,1,1"

    typedef struct tic {

      float ask;
      float bid;
      float close;
    }tic, *ticP;
    typedef struct pElem {
      char       * k;
      char       * v;
    }pElem, *pElemP;

    void fillFields(ticP t, pElemP p)
    {
      if (strcmp( ASK, p->k)==0)
      {
        printf ("ASK %s\n", p->v);
        t->ask = atof(p->v);
      }
      if (strcmp( BID, p->k)==0)
      {
        printf ("BID %s\n", p->v);
        t->bid = atof(p->v);
      }
      if (strcmp( CLOSE, p->k)==0)
      {
        printf("CLOSE >>>%sv)    ;
        t->close =  atof (p->v);
      }
    }

Solution

I like to go data driven, so I can write a single loop for processing. First, let's factor out the three if blocks that are so similar into a helper function:

void fillField(pElemP p, char* match, char* message, float* pf)
{
    if (strcmp(match, p->k)==0)
    {
        printf(message, p->v);
        *pf = atof(p->v);
    }
}
void fillFields(ticP t, pElemP p)
{
    fillField(p, ASK, "ASK %s\n", &t->ask);
    fillField(p, BID, "BID %s\n", &t->bid);
    fillField(p, CLOSE, "CLOSE >>>%sclose);
}


Now that we have this split out, we're repeating a lot less code, and have identified what information we'd need in order to make this data driven. I'm not sure going pure data driven is much better in this case, but let's look at one way it might be done:

struct FieldSpec {
    char *match;
    char *message;
    int offset;
};

struct FieldSpec field_specs[] = {
    { ASK, "ASK %s\n", offsetof(tic, ask) },
    { BID, "BID %s\n", offsetof(tic, bid) },
    { CLOSE, "CLOSE >>>%s<<<\n", offsetof(tic, close) },
};

void fillFields(ticP t, pElemP p)
{
    for (int i = 0; i < sizeof(field_specs) / sizeof(*field_specs); ++i)
    {
        fillFieldBySpec(field_specs[i], p, t);
    }
}


I'll leave the implementation of fillFieldBySpec up to you if you want to go this route (feel free to put it inside the for loop instead of its own function). I don't think it helps significantly over the first refactoring, and the gynmastics to use the value from offsetof aren't pretty.

Code Snippets

void fillField(pElemP p, char* match, char* message, float* pf)
{
    if (strcmp(match, p->k)==0)
    {
        printf(message, p->v);
        *pf = atof(p->v);
    }
}
void fillFields(ticP t, pElemP p)
{
    fillField(p, ASK, "ASK %s\n", &t->ask);
    fillField(p, BID, "BID %s\n", &t->bid);
    fillField(p, CLOSE, "CLOSE >>>%s<<<\n", &t->close);
}
struct FieldSpec {
    char *match;
    char *message;
    int offset;
};

struct FieldSpec field_specs[] = {
    { ASK, "ASK %s\n", offsetof(tic, ask) },
    { BID, "BID %s\n", offsetof(tic, bid) },
    { CLOSE, "CLOSE >>>%s<<<\n", offsetof(tic, close) },
};

void fillFields(ticP t, pElemP p)
{
    for (int i = 0; i < sizeof(field_specs) / sizeof(*field_specs); ++i)
    {
        fillFieldBySpec(field_specs[i], p, t);
    }
}

Context

StackExchange Code Review Q#36774, answer score: 6

Revisions (0)

No revisions yet.