Computing mean and median

Question

I have written a program to compute mean and median. Please review my code and give suggestions.

I have a confusion about this temps[temps.size()/2] when temps.size() is an odd number. How does the "vector::operator[]" handle floating point numbers? From the behavior in my code, I guessed it rounds to the nearest integer.

Note: I am self learning programming by reading "Programming: Principles, and Practice" by Bjarne Stroustrup. Currently, I am on chapter 4.

#include <iostream>
#include <vector>
#include <algorithm>

int main()
{
    std::vector<double> temps;          // temperatures
    for (double temp; std::cin>>temp; ) // read into temp
        temps.push_back(temp);      // put temp into vector

    // Compute Mean temperature:
    double sum = 0;
    for (int x : temps) sum += x;
    std::cout << "Average temperature: " << sum/temps.size() << '\n';

    // sort temperatures
    std::sort(temps.begin(),temps.end());

    // Compute Median temperature
    if(temps.size()%2==1) //Number of elements are odd
    {
        std::cout << "Median temperature: " << temps[temps.size()/2] << '\n';
    }
    else // Number of elements are even
    {
        int index = temps.size()/2;
        std::cout<< "Median temperature: " << (temps[index-1] + temps[index])/2;
    }

}

Answer 1

I have a confusion about this temps[temps.size()/2] when temps.size() is an odd number. How does the "vector::operator[]" handle floating point numbers?

There are well defined rules to implicit conversions. temps.size() returns an unsigned integral type that represents the total count of elements in the container. Dividing an unsigned integral by 2 (a signed integral), the implicit conversion rules for mixed expressions promotes the signed 2 to be unsigned. Division between two unsigned integrals results in an unsigned integral and any remaining fractional part is truncated. So \$3/2 = 1\$ and the remaining \$.5\$ is truncated and discarded.

---

    std::vector<double> temps;
    for (double temp; std::cin>>temp; )
        temps.push_back(temp);
    // temps = ???

Be aware of the possible states a type may represent. What happens if someone runs the program and inputs no data? Boom. The mean cannot be calculated because division-by-zero is undefined behavior. The median cannot be calculated as an empty set is consider an even-sized set. temps[0-1] accesses results in an out-of-bound access, which is also undefined behavior. Explicitly assert/validate your pre- and post-conditions. Do not rely on assumptions.

---

    for (int x : temps) sum += x;

Use auto to avoid redundant repetition of type names. temps represents a container of doubles. You define x to be of type int, which results in a narrowing conversion that's likely to be a source of a bug (Test! Test! Test!). The compiler already knows what type x should be. Use auto and let the compiler do its job.

---

Don't state in comments what can be clearly stated through code. When programming, the code states what is being done. If you need to provide/document any reasoning regarding your intent, use comments then.

Write functions. Lots of functions. You've already split your program into single logical operations via comments. Break up that monolithic main into parameterized functions.

int main() {
    auto temperatures = read_temperatures(std::cin);
    const auto mean_temperature = mean(temperatures);
    const auto median_temperature = median(temperatures);

    report(std::cout, mean_temperature, median_temperature);
}

Now you can write the short and simple functions that are easy to read, test, and maintain.

Get familiar with the standard algorithms found in <algorithm> and <numeric>. Do you really need to sort the entire data set? Would partial sorting (std::nth_element) suffice? For even-sized data sets, you could pair up std::nth_element with std::min_element to find the two values. As Vnp mentioned, std::accumulate is a candidate for summing up a range.

---

Use strongly defined types to represent your temperatures. A raw double representing temperatures is not safe. Numeric types that are conceptually incompatible may cause catastrophic mathematical errors. The Mars Climate Orbiter failure is a famous catastrophe caused by taking an impulse result from the US system and using that result in SI calculations without any conversions. So if you want to store temperatures, are you representing Celsius, Fahrenheit, Kelvin, or Rankine? Note - You'll need an understanding of programming with objects to accomplish this. If you haven't learned about objects yet, keep it in mind for when you do encounter them.

Answer 2

• To address your confusion, there is no floating point values. Both temps.size() and 2 are integrals, so the division is an integral division.

• To elaborate on the point above, you can avoid testing for parity by always computing two indices:

      ix1 = (temps.size() - 1) / 2;
      ix2 = temps.size() / 2;
      median = (temps[ix1] + temps[ix2]) / 2;
  

  I am not advocating it as a best solution, but it is worth effort to see how those indices align nicely regardless of parity.

• I strongly recommend to separate computation and output.

• I strongly recommend to not put everything in main, but factor computations out into the distinct mean and median functions.

• I don't know how much is already covered by chapter 4, but since you've already #include <algorithm>, you may want to investigate std::accumulate.

• Kudos for not using namespace std.

Answer 3

Concerning your question: When dividing an int by an int, or when converting double to int, the result is always an int. This means that the result is being rounded down, never up. The part after the floating point is just stripped away.

I suggest the following improvements:

#include <iostream>
#include <vector>
#include <algorithm>

int main()
{

Use expressive names to create self-documenting code. If a comment is needed to explain a variable, use a different name:

    std::vector<double> temperatures;

Comment blocks of code from the developer/user perspective, not single lines from the technical viewpoint:

    // Fill temperatures with user input.
    for (double temperature; std::cin>>temperature; )
    {
        temperatures.push_back(temperature);
    }

Also I'm not sure whether temp was intended to mean temperature or temporary (which would be the common usage of the abbreviation). But both would fit here, so let's stay in our domain view.

Use local variables to reuse calculated values and shorten lines of code. Good variable names can abstract details and increase readability:

    int number_of_temperatures = temperatures.size();

    // Calculate mean temperature.
    // (Maybe this should be a function instead.
    // The function name would replace the comment and shorten the code.)
    double sum = 0;
    for (int x : temperatures) sum += x;
    double mean_temperature = sum / number_of_temperatures;

Don't comment lines where the code already explains itself:

    std::sort(temperatures.begin(),temperatures.end());

Again we use intermediate results and save them into variables. This increases readability, and reduces the need for comments.

    // Calculate median temperature.
    bool even_amount = (number_of_temperatures % 2 == 0);
    int odd_amount_index = number_of_temperatures / 2;
    int even_amount_median = temperatures[odd_amount_index - 1] + temperatures[odd_amount_index]) / 2;

    double median_temperature = even_amount ?
                                even_amount_median : 
                                temperatures[odd_amount_index];

First we calculate, then we output the results:

    std::cout << "Average temperature: " << mean_temperature << "\n";
    std::cout<< "Median temperature: " << median_temperature << "\n";

}

Answer 4

It isn't necessary to completely sort the collection to compute the median.

We can get away with merely partitioning the data, using std::partition(). Then take the highest value from the first half; for even-length data, average that with the lowest value from the second half.

The algorithmic complexity is then still O(n log n), but with a greatly reduced constant factor.

Answer 5

To add to what others says, in your version of the median calculation you can simplify to:

int index = temps.size()/2;
double median = temps[index];

// if size is pair, compute the average with the t° before
if !(temps.size() % 2) {
    median = (median + temps[index-1])/2;
}
std::cout << "Median temperature: " << median << '\n';