Calculate the centroid of a collection of complex numbers

Question

In working on another problem, one component I needed was to calculate the centroid of a collection of complex objects. The well-known way to calculate this is to simply average the real and imaginary parts. To accommodate either std::complex<double> or std::complex<float>, I have created the following code as a template. Everything works as expected.

centroid.cpp

#include <iostream>
#include <complex>
#include <vector>
#include <numeric>

template<class ComplexIterator> 
typename ComplexIterator::value_type centroid(ComplexIterator a, ComplexIterator b) {
    return std::accumulate(a, b, typename ComplexIterator::value_type{0})
            / typename ComplexIterator::value_type{b-a};
}

int main() {
    std::vector<std::complex<float>> points{ {4,5}, {30,6}, {20,25} };
    std::cout << centroid(points.begin(), points.end()) << "\n";
}

Sample output:

(18, 12)

My questions

1. The compiler rightly warns that in the calculation of the denominator in the template, there is a narrowing conversion. I can't think of a way to elegantly handle that. Should I just ignore it?
2. If given an empty set, the code returns a value of (-nan, -nan) which works for me, but for general use, should I throw an exception instead?
3. Should I use std::enable_if or C++20 requires to constrain the function to require only floating point numbers?
4. I thought about calling it average because technically, it would happily compute the average of, say, a std::vector<float> but for my purposes, centroid seemed apt. What do you think of that choice?

Any other ways to improve this would be good as well.

Answer 1

There's too little to review...

It's not really a good practice to write function that accepts two iterators. Many STL functions work this way but it isn't convenient. It should accept a range instead. That's why ranges proposal was added to C++20.

centroid isn't a good name as even people well familiar with geometry might not remember this term. And it gets even more confusing as you use it with complex numbers because in programming complex numbers are usually used to represent 2d rotations rather than points in 2d space. You should use word mean or average instead - as a similar routine can just as easily compute arithmetic mean for whatever.

I'd put an assert to deal with division by zero. Let caller figure out what to do with no element case. And you definitely don't want to deal with nans.

Should I use std::enable_if or C++20 requires to constrain the function to require only floating point numbers?

Don't bother. And there is no full C++20 implementation - or even one remotely close fully functioning.

Answer 2

Taking all of the suggestions from the other reviews, I think it's quite improved so I wanted to thank the reviewers and show the final version:

#include <iostream>
#include <complex>
#include <vector>
#include <numeric>
#include <concepts>
#include <ranges>
#include <array>
#include <iterator>

template<typename T> 
concept ComplexOrFloat = std::floating_point<T>
    || std::floating_point<typename T::value_type>;

template<typename R> 
requires std::ranges::range<R> 
    && ComplexOrFloat<std::ranges::range_value_t<R>>
constexpr std::ranges::range_value_t<R> average(R range) {
    if (std::ranges::size(range) == 0) 
        throw std::domain_error("Cannot take average of zero-length range");
    using T = std::ranges::range_value_t<R>;
    return std::accumulate(std::ranges::begin(range), std::ranges::end(range), T{0}) 
        / (T)(std::ranges::size(range));
}

int main() {
    using namespace std::complex_literals;
    constexpr std::array<std::complex<double>,3> points{{ {4,5}, {30,6}, {20,25} }};
    std::cout << average(std::views::all(points)) << "\n";

    constexpr std::array<std::complex<double>,3> points2{
        {  24.0 + 7.0i, 22.0 + 9.0i, 8.0 + 20.0i }
    };
    std::cout << average(std::views::all(points2)) << "\n";

    constexpr double numbers[]{1, -2, 3, 7, -8};
    std::cout << average(std::views::all(numbers)) << "\n";

    const std::vector<float> numbers2{-7, -2, 3, 7, -8};
    std::cout << average(std::views::all(numbers2)) << "\n";

    const std::vector<float> mt{};
    try {
        std::cout << average(std::views::all(mt)) << "\n";
    } catch (std::domain_error& err) {
        std::cerr << "ERROR: " << err.what() << "\n";
    }

#if 0
    int intnumbers[]{1, -2, 3, 7, -8};
    // won't compile because an `int` is not floating point or complex floating point
    std::cout << average(std::views::all(intnumbers)) << "\n";
#endif
}

I'm using C++20 concepts to avoid trying to take an average of a range of ints and returning an int.

Answer 3

I'm fine with (-nan. -nan) and calling it centroid, but I'm not strongly tied to those opinions. I don't have enough experience with c++20 to weigh in on std:enable_if.

Other improvements:

• the template should be constexpr.