Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
  • stdlib.h doesn’t exist in C++. The correct header is cstdlib although most (but not all!) compilers will also find the former.

  • using namespace std is generally discouraged since it may cause prolific name clashes. Instead, import just the names you need, at small scope (i.e. not at file level but in individual functions).

  • Parenthetical: if there is any chance that this code may be reused (not likely, since it’s a toy example) use SI units, not Imperial ones, for the calculation. If you want to support Imperial input, convert the units to SI on input, and convert back before output, but use standard units internally. Incidentally, this also makes the calculation easier.

  • Your bmiNumber calculation is wrong: the function claims that it has inches as input. But this is a lie: the input is actually inches squared. This is a recipe for bugs.

  • What does the mF parameter in chart exactly do?

  • The calculation of totalinches might be better off in function which takes whole feet and inches as arguments.

  • Extensive text data, such as that printed in chart and history are probably better off in a separate file.

  • The main function is crowded.

But more importantly, you are using the C++ stream library as an interactive input library, and that’s not what it was designed for, and doesn’t work well for. Unfortunately, C++ doesn’t ship with an interactive console input library. If you insist on the interactivity, use a library such as Curses for C++ Curses for C++.

However, console applications are usually designed to be non-interactive, and controlled by command line arguments or config files.

If this were a real program, I’d probably design it to be used as follows:

bmi --male --size=6ft2 --weight=160lb

Or, to output the history:

bmi --history

Once again, C++ does’t offer great support for this in its standard library, but once again, there are libraries for that. The default on Unix systems os getopt but for C++ a better library is Boost.ProgramOptions.

The real secret to C++ is to use libraries for everything but the most trivial tasks. Bjarne Stroustrup, the inventor of C++, once said that,

Without a good library, most interesting tasks are hard to do in C++; but given a good library, almost any task can be made easy.

  • stdlib.h doesn’t exist in C++. The correct header is cstdlib although most (but not all!) compilers will also find the former.

  • using namespace std is generally discouraged since it may cause prolific name clashes. Instead, import just the names you need, at small scope (i.e. not at file level but in individual functions).

  • Parenthetical: if there is any chance that this code may be reused (not likely, since it’s a toy example) use SI units, not Imperial ones, for the calculation. If you want to support Imperial input, convert the units to SI on input, and convert back before output, but use standard units internally. Incidentally, this also makes the calculation easier.

  • Your bmiNumber calculation is wrong: the function claims that it has inches as input. But this is a lie: the input is actually inches squared. This is a recipe for bugs.

  • What does the mF parameter in chart exactly do?

  • The calculation of totalinches might be better off in function which takes whole feet and inches as arguments.

  • Extensive text data, such as that printed in chart and history are probably better off in a separate file.

  • The main function is crowded.

But more importantly, you are using the C++ stream library as an interactive input library, and that’s not what it was designed for, and doesn’t work well for. Unfortunately, C++ doesn’t ship with an interactive console input library. If you insist on the interactivity, use a library such as Curses for C++.

However, console applications are usually designed to be non-interactive, and controlled by command line arguments or config files.

If this were a real program, I’d probably design it to be used as follows:

bmi --male --size=6ft2 --weight=160lb

Or, to output the history:

bmi --history

Once again, C++ does’t offer great support for this in its standard library, but once again, there are libraries for that. The default on Unix systems os getopt but for C++ a better library is Boost.ProgramOptions.

The real secret to C++ is to use libraries for everything but the most trivial tasks. Bjarne Stroustrup, the inventor of C++, once said that,

Without a good library, most interesting tasks are hard to do in C++; but given a good library, almost any task can be made easy.

  • stdlib.h doesn’t exist in C++. The correct header is cstdlib although most (but not all!) compilers will also find the former.

  • using namespace std is generally discouraged since it may cause prolific name clashes. Instead, import just the names you need, at small scope (i.e. not at file level but in individual functions).

  • Parenthetical: if there is any chance that this code may be reused (not likely, since it’s a toy example) use SI units, not Imperial ones, for the calculation. If you want to support Imperial input, convert the units to SI on input, and convert back before output, but use standard units internally. Incidentally, this also makes the calculation easier.

  • Your bmiNumber calculation is wrong: the function claims that it has inches as input. But this is a lie: the input is actually inches squared. This is a recipe for bugs.

  • What does the mF parameter in chart exactly do?

  • The calculation of totalinches might be better off in function which takes whole feet and inches as arguments.

  • Extensive text data, such as that printed in chart and history are probably better off in a separate file.

  • The main function is crowded.

But more importantly, you are using the C++ stream library as an interactive input library, and that’s not what it was designed for, and doesn’t work well for. Unfortunately, C++ doesn’t ship with an interactive console input library. If you insist on the interactivity, use a library such as Curses for C++.

However, console applications are usually designed to be non-interactive, and controlled by command line arguments or config files.

If this were a real program, I’d probably design it to be used as follows:

bmi --male --size=6ft2 --weight=160lb

Or, to output the history:

bmi --history

Once again, C++ does’t offer great support for this in its standard library, but once again, there are libraries for that. The default on Unix systems os getopt but for C++ a better library is Boost.ProgramOptions.

The real secret to C++ is to use libraries for everything but the most trivial tasks. Bjarne Stroustrup, the inventor of C++, once said that,

Without a good library, most interesting tasks are hard to do in C++; but given a good library, almost any task can be made easy.

Source Link
Konrad Rudolph
  • 6.7k
  • 23
  • 35
  • stdlib.h doesn’t exist in C++. The correct header is cstdlib although most (but not all!) compilers will also find the former.

  • using namespace std is generally discouraged since it may cause prolific name clashes. Instead, import just the names you need, at small scope (i.e. not at file level but in individual functions).

  • Parenthetical: if there is any chance that this code may be reused (not likely, since it’s a toy example) use SI units, not Imperial ones, for the calculation. If you want to support Imperial input, convert the units to SI on input, and convert back before output, but use standard units internally. Incidentally, this also makes the calculation easier.

  • Your bmiNumber calculation is wrong: the function claims that it has inches as input. But this is a lie: the input is actually inches squared. This is a recipe for bugs.

  • What does the mF parameter in chart exactly do?

  • The calculation of totalinches might be better off in function which takes whole feet and inches as arguments.

  • Extensive text data, such as that printed in chart and history are probably better off in a separate file.

  • The main function is crowded.

But more importantly, you are using the C++ stream library as an interactive input library, and that’s not what it was designed for, and doesn’t work well for. Unfortunately, C++ doesn’t ship with an interactive console input library. If you insist on the interactivity, use a library such as Curses for C++.

However, console applications are usually designed to be non-interactive, and controlled by command line arguments or config files.

If this were a real program, I’d probably design it to be used as follows:

bmi --male --size=6ft2 --weight=160lb

Or, to output the history:

bmi --history

Once again, C++ does’t offer great support for this in its standard library, but once again, there are libraries for that. The default on Unix systems os getopt but for C++ a better library is Boost.ProgramOptions.

The real secret to C++ is to use libraries for everything but the most trivial tasks. Bjarne Stroustrup, the inventor of C++, once said that,

Without a good library, most interesting tasks are hard to do in C++; but given a good library, almost any task can be made easy.

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /