Calculations not being displayed, everything else working fine?

Hi everyone,

I'm writing a program that should take in a number, e.g. 12345, and convert it to what my professor calls engineering notation, e.g. 12k. My code is probably longer than it needs to be, but I'm really focusing on readability and using each function for one purpose. My program successfully checks the input and, if invalid, notifies the user accordingly. However, upon receiving valid input, it just prints a newline and then prompts the user for a new number. Here is my code:

Code:

---


#include <stdio.h>
#include <math.h>
#include <ctype.h>

#define MIN 1
#define MAX pow(10, 15)

int fnCheck(long double);
char * fnCalculateFor(long double);
int fnReactTo(long double);

int fnCheck(long double d) {

  int isDouble = scanf(" %Lf", &d);
  int isWithinBounds = d >= MIN && d < MAX;
  int status = d ? (isDouble + isWithinBounds) * isDouble : 3;
  return status;
 
}

char * fnCalculateFor(long double d) {

  char prefixes[7] = {'_', 'k', 'M', 'G', 'T', 'P'};
  static char out[10];
  long double n;
 
  for (int i = 5; i >= 0; i--) {
    if ((n = d / pow(10, i * 3)) >= 1) {
      snprintf(out, sizeof(out), " = %d%c\n", (int) n, prefixes[i]);
      break;
    }
  }
  return out;
 
}

int fnReactTo(long double d) {
 
  int inDex = fnCheck(d);
  int g = 1;
 
  switch (inDex) {
    case 0:
      puts("\nInvalid input. Shutting down...");
    case 3:
      g--;
      break;
    case 2:
      puts(fnCalculateFor(d));
      break;
    case 1:
      puts("\nNumber must be in range 1 ... 1e15.");
  }
  return g;
 
}

int main(void) {
 
  long double input;
  int goAhead;

  puts("Welcome to the homework guide. At any time enter 0 to exit the application.  User input is constrained between 1 and 999,999,999,999,999.\n");
 
  do {
    puts("Please enter a number: ");
    goAhead = fnReactTo(input);
  } while (goAhead);
 
  puts("\nThank you for using the application.");
  return 0;
 
}



---

I don't know whether the calculations are even being done, or if the error is somewhere else in the program. As mentioned in previous posts, I am pretty much brand new to C and I don't necessarily fully understand some concepts even if I apply them here, e.g. pointers.

Any help appreciated! :biggrin:

Do you notice that your posted code looks bad? That's because you didn't post it as plain text.

Quote:

---

My code is probably longer than it needs to be, but I'm really focusing on readability and using each function for one purpose.

---

Longer is not generally more readable. Consider Mark Twain's famous quote: "I apologize for such a long letter - I didn't have time to write a short one."

Your names are pretty bad. There's certainly no need to prefix functions with fn. And "goAhead", "reactTo" ... what?

There's no need to pass "input" (uninitialized!) into reactTo. Just use a local variable in the function.

What is the output supposed to be for an input of 12999? Is it supposed to be truncated as 12k, or rounded up as 13k? The following assumes truncation.

Code:

---

#include <stdio.h>
 
int main() {
    char prefixes[] = {
        'k', 'M', 'G', 'T', 'P', 'E'
    };
 
    printf("Enter a number: ");
    unsigned long n;
    scanf("%lu", &n);
 
    int i = 0;
    for ( ; n >= 1000; ++i)
        n /= 1000;
 
    printf("%lu", n);
    if (i > 0) putchar(prefixes[i - 1]);
    putchar('\n');
 
    return 0;
}

---

Your code is much shorter, but doesn't handle invalid input, does not define boundaries and only allows input once. The fn prefix on function names was at the suggestion of my professor. goAhead tells the main loop to go ahead to the next iteration. reactTo() defines the protocol for reacting to its argument. Truncation is intended, as opposed to rounding. This does not solve the problem.

Quote:

---

any help appreciated!

---

Guess not.
Good luck with that.

It's such a crybaby thing to whine that I didn't solve every single one of your requirements.
Here you go:

Code:

---

#include <stdio.h>
 
int main() {
    char prefixes[] = { 'k', 'M', 'G', 'T', 'P' };
 
    while (1) {
        unsigned long n;
 
        while (1) {
            printf("Enter a number: ");
            if (scanf("%lu", &n) != 1) {
                printf("Invalid input. Try again.\n");
                for (int c; (c = getchar()) != EOF && c != '\n'; ) ;
            }
            else if (n <= 1000000000000000)
                break;
            printf("Too large. Try again.\n");
        }
 
        if (n == 0) break;
 
        int i = 0;
        for ( ; n >= 1000; ++i)
            n /= 1000;
 
        printf("%lu", n);
        if (i > 0) putchar(prefixes[i - 1]);
        putchar('\n');
    }
 
    return 0;
}

---

I apologize if answering your condescending, rhetorical questions upset you. You posted a fairly useless code snippet that seemed only to be there as a 'Hey look at me I can do what you did in 21 lines' kind of thing, which would be in accordance with the general tone of your reply. I never asked anyone to rewrite it, I was only trying to find what went wrong with the code I had already written. Can't believe you're calling me a crybaby, I would never call you a jerk...

Quote:

---

Originally Posted by C-UL8R

The fn prefix on function names was at the suggestion of my professor.

---

You probably misinterpreted your professor: perhaps it was a suggestion for a metasyntactic name like foo, bar, baz, or func that might be used when we want to name a function so as to demonstrate something about language syntax. You shouldn't be using it as a function name prefix as it clutters the name: we can already see that it is a function from either the declaration, or usage, or verb-based naming, or simply by checking the type with the code introspection capabilities of modern IDEs.

This seems pretty cryptic and incorrect:

Code:

---

int fnCheck(long double d) {

  int isDouble = scanf(" %Lf", &d);
  int isWithinBounds = d >= MIN && d < MAX;
  int status = d ? (isDouble + isWithinBounds) * isDouble : 3;
  return status;
 
}

---

It would be much easier to understand if you used named constants and didn't try to compute the status code:

Code:

---

enum status_code {
    INPUT_SUCCESS,
    INPUT_END,
    INPUT_READ_ERROR,
    INPUT_RANGE_ERROR
};

enum status_code readNumber(long double *num) {
    if (scanf(" %Lf", num) != 1) {
        return INPUT_READ_ERROR;
    }
    if (*num == 0) {
        return INPUT_END;
    }
    if (*num < MIN || *num > MAX) {
        return INPUT_RANGE_ERROR;
    }
    return INPUT_SUCCESS;
}

---

Now you can use it like this in the main function:

Code:

---

int end = 0;
do {
    puts("Please enter a number: ");
    long double num;
    switch (readNumber(&num)) {
    case INPUT_SUCCESS:
        puts(fnCalculateFor(num));
        break;
    case INPUT_READ_ERROR:
        puts("\nInvalid input. Shutting down...");
    case INPUT_END:
        end = 1;
        break;
    case INPUT_RANGE_ERROR:
        puts("\nNumber must be in range 1 ... 1e15.");
    }
} while (!end);

---

As for why:

• Named constants make it easier to understand the code than magic numbers.
• Your fnCheck function was primarily reading input rather than merely checking it: the check is input validation, important but secondary to the main goal of reading input. Hence, readNumber is a better name.
• Your fnCheck function provided a long double parameter, which meant that any changes to that variable was merely local. readNumber provides a pointer to long double parameter so that reading into that variable changes the variable from the caller, which is what you want. An alternative as john.c mentioned is "just use a local variable in the function", but for what it looks like you had in mind (output parameter + status code return), that doesn't work (unless you assign the local variable to the output parameter).
• Naming issues aside, fnReactTo isn't entirely a bad idea as it can be useful to abstract away the body of the main loop. In this case though, your main function is already so small that it doesn't help.
• I renamed g to end (the descriptive name makes its purpose clearer) and inverted its use as a flag, and because it is a flag I simply set it to 0 or 1 rather than trying to do a fancy and unnecessary decrement. You can also #include <stdbool.h> and use bool, true, and false instead of int, 1, and 0 respectively.

I would normally be a little wary about comparing floating point numbers with == but in the case of 0 as read by scanf you probably won't run into floating point inaccuracy.

Thank you laserlight! Very informative. My professor does indeed use a type prefix in the names of all functions and variables (e.g. int intA; char charC). He mentioned that it was helpful in earlier programming styles, but it may be unnecessary now.