7
\$\begingroup\$

I would like my program to be reviewed. I had to write a simple program that calculates the number of molecules in a hydrocarbon.

  • 1 carbon atom has 12 AMU.
  • 1 hydrogen atom has 1 AMU.

#include <iostream>
using namespace std;



int main()
{
    cout << "Enter mass of hydrocarbon (in grams)\n"
            "followed by the number of carbon atoms\n"
            "followed by the number of hydrogen atoms\n"
            "(e.g. 10.5 2 6): " << flush;

    float mass;
    float carbonAtoms;
    int hydrogenAtoms;
    cin >> mass >> carbonAtoms >> hydrogenAtoms;

    long formulaWght = (carbonAtoms * 12) + (hydrogenAtoms * 1);


    double molecules = (mass / formulaWght) * 6.02e23;



    cout << mass << " grams of hydrocarbon\nwith "
            << carbonAtoms << " carbon atom(s) and "
               << hydrogenAtoms << " hydrogen atom(s)\ncontains "
                  << molecules << " molecules" << endl;

    return 0;
}
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$
  1. Do not use using namespace std. Read this
  2. Instead of using "magic numbers" in your program. Use symbolic constants. CARBON_AMU and HYDROGEN_AMU should be ints.

    const int CARBON_AMU= 12;
    const int HYDROGEN_AMU = 1;
    const double AVOGADRO = 6.02214141070409084099072e23;
    
  3. Utilize functions in your program, instead of initializing a variable with an expression:

    long formulaWeight(int carbonAtoms, int hydrogenAtoms){
        return((carbonAtoms * CARBON_AMU) + (hydrogenAtoms * HYDROGEN_AMU));
    }
    
    double numMolecules(float mass, long formulaWeight)
    {
        return((mass / formulaWeight) * AVOGADRO);
    }
    
  4. (e.g. 10.5 2 6): " << flush;

    I don't see any advantage in using std::flush. You can remove it.

  5. Fix your indentation. This is much easier to read:

    cout << mass << " grams of hydrocarbon\nwith "
         << carbonAtoms << " carbon atom(s) and "
         << hydrogenAtoms << " hydrogen atom(s)\ncontains "
         << molecules << " molecules" << endl;
    
\$\endgroup\$
4
  • 1
    \$\begingroup\$ You could also mention that carbonAtoms should be an int. \$\endgroup\$
    – Jamal
    Commented Oct 11, 2014 at 3:50
  • \$\begingroup\$ Also, I think the first indentation example is easier to read. The second one is better aligned, but is kinda mixed up. \$\endgroup\$
    – Jamal
    Commented Oct 11, 2014 at 3:52
  • \$\begingroup\$ in simple code like this, I don't see any issue use 'using namespace std;' \$\endgroup\$
    – zinking
    Commented Oct 11, 2014 at 4:10
  • \$\begingroup\$ It can become a bad habit and should be stopped early on. This question is tagged beginner too. \$\endgroup\$ Commented Oct 11, 2014 at 4:14
2
\$\begingroup\$

Some small addendums.

All the parentheses on these lines are unnecessary, because in C++ (and most languages, if not all) multiplication has higher precedence than addition:

long formulaWght = (carbonAtoms * 12) + (hydrogenAtoms * 1);
double molecules = (mass / formulaWght) * 6.02e23;

And by the way, why multiply by 1?

This is equivalent:

long formulaWght = carbonAtoms * 12 + hydrogenAtoms;
double molecules = mass / formulaWght * 6.02e23;

In addition to the recommendation by @EngieOP to reindent the last cout, I would recommend to rearrange the lines in a way that the embedded \n characters appear at the end of code lines, like they will be in the output, like this:

cout << mass << " grams of hydrocarbon\n"
     << "with " << carbonAtoms << " carbon atom(s) and "
     << hydrogenAtoms << " hydrogen atom(s)\n"
     << "contains " << molecules << " molecules" << endl;

Instead of using namespace std, if you want to be able to reference simply cout and endl, you could do this acceptable way:

using std::cout;
using std::endl;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.