patterncMinor
Parsing comma-separated floats and semicolon-delimited commands
Viewed 0 times
commadelimitedcommandssemicolonseparatedparsingandfloats
Problem
I wrote a
Function 1:
Function 2:
```
bool Receiver::parse_pid_conf(char* buffer) {
if(m_pHalBoard == NULL) {
return false;
}
else if(m_rgChannelsRC[2] > RC
cstring parser. It has to work with a relatively wide amount of arguments (usually 3 or 4, but maybe in future with a different amount), which are separated by an , or ;. My wish is to make at least Function 1 less static and save some code. However, I don't really want to use dynamic memory allocation (just if it not to avoid and would bring a benefit). The whole code should run on ATMega2560. This is why I cannot use higher library functions.Function 1:
float *Receiver::parse_pid_substr(char* buffer) {
static float pids[8];
memset(pids, 0, 8*sizeof(float) );
char cI[32], cII[32], cIII[32], cIV[32], cV[32], cVI[32], cVII[32], cVIII[32];
size_t i = 0, c = 0, p = 0;
for(; i < strlen(buffer); i++) {
if(buffer[i] == '\0') {
break;
}
else if(buffer[i] != ',') {
switch(p) {
case 0:
cI[c] = buffer[i];
cI[c+1] = '\0';
break;
case 1:
cII[c] = buffer[i];
cII[c+1] = '\0';
break;
case 2:
cIII[c] = buffer[i];
cIII[c+1] = '\0';
break;
case 3:
cIV[c] = buffer[i];
cIV[c+1] = '\0';
break;
case 4:
cV[c] = buffer[i];
cV[c+1] = '\0';
break;
case 5:
cVI[c] = buffer[i];
cVI[c+1] = '\0';
break;
case 6:
cVII[c] = buffer[i];
cVII[c+1] = '\0';
break;
case 7:
cVIII[c] = buffer[i];
cVIII[c+1] = '\0';
break;
}
c++;
} else {
p++;
c = 0;
continue;
}
}
pids[0] = atof(cI);
pids[1] = atof(cII);
pids[2] = atof(cIII);
pids[3] = atof(cIV);
pids[4] = atof(cV);
pids[5] = atof(cVI);
pids[6] = atof(cVII);
pids[7] = atof(cVIII);
return pids;
}Function 2:
```
bool Receiver::parse_pid_conf(char* buffer) {
if(m_pHalBoard == NULL) {
return false;
}
else if(m_rgChannelsRC[2] > RC
Solution
Two-dimesional array
Your first function can be aggressively reduced by replacing
In the following piece of code:
The
Zero-initialization
Instead of using
Names
The names you use are all but explicit. While
NOTE: Hadn't it been for the
Your first function can be aggressively reduced by replacing
cI and friends by a 2D array:float *Receiver::parse_pid_substr(char* buffer) {
static float pids[8];
memset(pids, 0, 8*sizeof(float) );
char rgcPIDS[8][32];
size_t i = 0, c = 0, p = 0;
for(; i < strlen(buffer); i++) {
if(buffer[i] == '\0') {
break;
}
else if(buffer[i] != ',') {
rgcPIDS[p][c] = buffer[i];
rgcPIDS[p][c+1] = '\0';
c++;
} else {
p++;
c = 0;
continue;
}
}
for (int j = 0 ; j < 8 ; ++j) {
pids[j] = atof(chars[j]);
}
return pids;
}continueIn the following piece of code:
} else {
p++;
c = 0;
continue;
}The
else clause is the last of your loop, and continue is the last instruction of that clause. You can remove continue: even without it, your function will still work and behave the same way as before.Zero-initialization
Instead of using
memset to zero-initialize your arrays, you can use the language zero-initialization for arrays whose bound is known at compile-time:char rgcPIDS[8][32] = { 0 };Names
The names you use are all but explicit. While
i is good as a loop iterator, c and p should have more explicit names.NOTE: Hadn't it been for the
Receiver::, I would have thought that your code was written in plain old C, and not in C++. Is it intentional?Code Snippets
float *Receiver::parse_pid_substr(char* buffer) {
static float pids[8];
memset(pids, 0, 8*sizeof(float) );
char rgcPIDS[8][32];
size_t i = 0, c = 0, p = 0;
for(; i < strlen(buffer); i++) {
if(buffer[i] == '\0') {
break;
}
else if(buffer[i] != ',') {
rgcPIDS[p][c] = buffer[i];
rgcPIDS[p][c+1] = '\0';
c++;
} else {
p++;
c = 0;
continue;
}
}
for (int j = 0 ; j < 8 ; ++j) {
pids[j] = atof(chars[j]);
}
return pids;
}} else {
p++;
c = 0;
continue;
}char rgcPIDS[8][32] = { 0 };Context
StackExchange Code Review Q#44867, answer score: 8
Revisions (0)
No revisions yet.