Implementation of the atof() function

Question

I am looking for feedback on coding style. I know this function already exists, and I would never use my version over a well tested one. I am just practicing.

double Clib_atof(char s[])
{
 double val, power, rtn;
 int i, sign;
 for (i = 0; isspace(s[i]); i++);
 sign = (s[i] == '-') ? -1 : 1;
 if (s[i] == '+' || s[i] == '-') ++i;
 for (val = 0.0; isdigit(s[i]); ++i) {
 val = 10.0 * val + (s[i] - '0');
 }
 if (s[i] == '.') {
 ++i;
 }
 for (power = 1.0; isdigit(s[i]); ++i) {
 val = 10.0 * val + (s[i] - '0');
 power *= 10.0;
 }
 rtn = (sign * val / (power));
 // Next work on exponent
 if (s[i] == 'e') {
 int j, esign; 
 int eval = 0;
 fprintf(stdout, "e found\n");
 for (j = i + 1; isspace(s[j]); ++j);
 esign = (s[j] == '-') ? -1 : 1;
 if (s[j] == '+' || s[j] == '-') ++j;
 for (; isdigit(s[j]); ++j) {
 eval = 10 * eval + (s[j] - '0');
 }
 fprintf(stdout, "eval = %d\n", eval);
 int l;
 for (l = 0; l < eval; l++) {
 (esign >= 0) ? (rtn *= 10) : (rtn /= 10);
 }
 }
 // Finally return the solution
 return rtn;
}

Answer 1

double Clib_atof(char s[])
{
 double val, power, rtn;
 int i, sign;
 for (i = 0; isspace(s[i]); i++);

Looks too much like the ; was an accident. I suggest using {/\*empty\*/} instead to make it clear you did it on purpose. Or I might write it as a while loop.

 sign = (s[i] == '-') ? -1 : 1;
 if (s[i] == '+' || s[i] == '-') ++i;
 for (val = 0.0; isdigit(s[i]); ++i) {
 val = 10.0 * val + (s[i] - '0');
 }

I dislike the use of a for loop here. The initializer condition doesn't really have any to do with the loop control. Repeating the loop until some condition becomes true feels like a while loop. With a for loop I think of iterating or counting not testing.

 if (s[i] == '.') {
 ++i;
 }
 for (power = 1.0; isdigit(s[i]); ++i) {
 val = 10.0 * val + (s[i] - '0');
 power *= 10.0;
 }
 rtn = (sign * val / (power));

I wouldn't use abbreviations like rtn and val. They make the code harder to read. I don't think any of those parens are necessary.

 // Next work on exponent
 if (s[i] == 'e') {
 int j, esign; 
 int eval = 0;

I look at this and think evaluate, but I don't think that is what you meant.

 fprintf(stdout, "e found\n");
 for (j = i + 1; isspace(s[j]); ++j);

Why would you allow space here? Why j? Why didn't you stick with i?

 esign = (s[j] == '-') ? -1 : 1;
 if (s[j] == '+' || s[j] == '-') ++j;
 for (; isdigit(s[j]); ++j) {
 eval = 10 * eval + (s[j] - '0');
 }

I note that this is pretty much the same as a previous block of code. Consider extracting the common bits into a function:

 fprintf(stdout, "eval = %d\n", eval);
 int l;
 for (l = 0; l < eval; l++) {
 (esign >= 0) ? (rtn *= 10) : (rtn /= 10);
 }

I suspect using a pow() function would be more fruitful.

 }
 // Finally return the solution
 return rtn;
}