Calculate total, minimum, maximum and mean from a set of distances I have only begun to learn C++ and am reading through the book, "Programming Principles and Practices Using C++". This bit of code is my attempt to complete exercise 3 at the end of chapter 4.

The actual task is:


  Read a sequence of double values into a vector. Think of each value as the distance between two cities along a given route. Compute and print the total distance (the sum of all distances). Find and print the smallest and greatest distance between two neighboring cities. Find and print the mean distance between two neighboring cities.


I am looking for any constructive criticism of my code. Keep in mind some things have not been covered. Even saying that, I still wouldn't mind hearing about what could be. Some things have been covered and I just don't implement them well.

    #include "std_lib_facilities.h"

int main()
{
    double ddistance = 0;
    double dtotaldist = 0;
    double dgreatest = 0;
    double dshortest = 0;
    char cgoagain;
    bool bgoagain = true;
    vector&lt;double&gt;vdistance;

    while (bgoagain) {
        cout &lt;&lt; "Enter a valid 'double' ";
        cin &gt;&gt; ddistance;


    if (cin) {
        vdistance.push_back(ddistance);
        dtotaldist += ddistance;
        dshortest = ddistance;

        for (int i = 0; i &lt; vdistance.size(); ++i)
            if ((vdistance[i]) &gt; dgreatest) {
                dgreatest = vdistance[i];
            }
        for (int i = 0; i &lt; vdistance.size(); ++i)
            if (vdistance[i] &lt; dshortest) {
                dshortest = vdistance[i];
            }

        std::cout &lt;&lt; "vdistance size is\t" &lt;&lt; vdistance.size() &lt;&lt; "\n";
        std::cout &lt;&lt; "dtotaldist size is\t" &lt;&lt; dtotaldist &lt;&lt; "\n";
        std::cout &lt;&lt; "The mean distance is\t" &lt;&lt; dtotaldist / vdistance.size() &lt;&lt; "\n";
        std::cout &lt;&lt; "The long distance is\t" &lt;&lt; dgreatest &lt;&lt; "\n";
        std::cout &lt;&lt; "The short distance is\t" &lt;&lt; dshortest &lt;&lt; "\n";
    }

    else {
        std::cout &lt;&lt; "That is not a valid value. Would you like to try again? 'y' / 'n' \n";
        std::cin.clear();
        std::cin.ignore(std::numeric_limits&lt;std::streamsize&gt;::max(), '\n');
        cin &gt;&gt; cgoagain;
        if (cgoagain == 'y' || cgoagain == 'Y') {
            bgoagain = true;
        }
        else{
            bgoagain = false;
        }
    }


    }
}
I guess std_lib_facilities is a header from the book? It's best to get used to including the standard library headers directly as soon as possible.

If std_lib_facilities contains using namespace std; note that this is bad practice.

In this case, we only need:

#include &lt;vector&gt;
#include &lt;iostream&gt;




"Hungarian" notation (adding type information to the names of variables), is not generally recommended. As such, the variable names should not be ddistance, cgoagain etc., just distance and goagain.

Using camel case or underscores may make them easier to read, and it's generally better to use whole words instead of abbreviations, e.g. totalDistance.



While C once had a requirement for all variables to be declared at the start of a block, C++ does not. Variables should be declared as close to their point of use as practical. As such, ddistance and cgoagain can be moved to just above where they are populated from std::cin.



Since you're storing the greatest and shortest distances, there's no need to iterate the whole vector of distances when a new value is entered. We can simply compare against the existing value. However, this won't work when initializing the greatest and shortest values to zero!

We can use another part of the standard library to initialize the greatest value to something that will always be smaller than a valid double input, and the shortest value to something that will always be larger. That way the first input will override both values successfully:

#include &lt;limits&gt;
...
double greatest = std::numeric_limits&lt;double&gt;::lowest();
double shortest = std::numeric_limits&lt;double&gt;::max();

...
        if (distance &gt; greatest)
            greatest = distance;

        if (distance &lt; shortest)
            shortest = distance;


Note that with this change, there's actually no need to keep a vector containing all the distances. We can just keep a count of the number of values entered and use that to calculate the mean.



Altered code:

#include &lt;vector&gt;
#include &lt;iostream&gt;
#include &lt;limits&gt;

int main()
{
    double totalDistance = 0;
    double greatest = std::numeric_limits&lt;double&gt;::lowest();
    double shortest = std::numeric_limits&lt;double&gt;::max();
    unsigned int distancesEntered = 0;

    while (true) {

        std::cout &lt;&lt; "Enter a valid 'double' ";

        double distance = 0;
        std::cin &gt;&gt; distance;

        if (std::cin) {

            ++distancesEntered;

            totalDistance += distance;

            if (distance &gt; greatest)
                greatest = distance;

            if (distance &lt; shortest)
                shortest = distance;

            std::cout &lt;&lt; "distancesEntered is\t" &lt;&lt; distancesEntered &lt;&lt; "\n";
            std::cout &lt;&lt; "totalDistance is\t" &lt;&lt; totalDistance &lt;&lt; "\n";
            std::cout &lt;&lt; "The mean distance is\t" &lt;&lt; totalDistance / distancesEntered &lt;&lt; "\n";
            std::cout &lt;&lt; "The greatest distance is\t" &lt;&lt; greatest &lt;&lt; "\n";
            std::cout &lt;&lt; "The shortest distance is\t" &lt;&lt; shortest &lt;&lt; "\n";
        }

        else {
            std::cout &lt;&lt; "That is not a valid value. Would you like to try again? 'y' / 'n' \n";
            std::cin.clear();
            std::cin.ignore(std::numeric_limits&lt;std::streamsize&gt;::max(), '\n');

            char goAgain = 0;
            std::cin &gt;&gt; goAgain;

            if (!(goAgain == 'y' || goAgain == 'Y')) {
                break;
            }
        }
    }
}