C++ STL列表计算平均值

Question

我将要纠正一些关于C++/STL的代码。不幸的是，我对于C++并没有很多经验，也不了解STL。尽管如此，我已经完成了大部分工作，但是下面这个函数仍然让我头疼：

C++源码：

double MyClass::CalculateAvg(const std::list<double> &list)
{
    double avg = 0;
    std::list<int>::iterator it;
    for(it = list->begin(); it != list->end(); it++) avg += *it;
    avg /= list->size();
}

C++ 头文件：

static double CalculateAvg(const std::list<int> &list);

这段文字很可能是用于计算列表中的平均值，但其中存在许多错误。我尝试在网络上寻找解决方案，但没有找到任何有用的信息。如果有人能帮助我解决问题，我将不胜感激。

更新： 感谢您的快速回复。接受的答案解决了我所有的问题。

- xsl

---

你应该至少返回结果："return avg"。 - Maurits Rijk

为什么你不在这里发布错误信息呢？你知道我们不是心理学家。 - jalf

Answer 1

几点建议：

1. 你没有返回任何东西。（加入 return avg;）
2. -> 操作符是用于指向对象的指针。你有一个列表的引用，因此使用 list.begin()，而不是 list->begin() （其他成员函数也是如此）。
3. 迭代器应该是 const_iterator，而不是 iterator。

无论哪种情况，您都应该只需要执行以下操作：

return std::accumulate(list.begin(), list.end(), 0.0) / list.size();

如果可能的话，还可以选择检查 list.size() == 0。

- Peter Alexander

---

对于std::accumulate，记得加1，要注意list.size()会遍历整个列表，所以不要调用两次。 - jk.

size() 在一些编译器上是一个常数时间操作，并且在 C++0x 中必须如此。 - jalf

我不确定为什么我用了"将会"而不是"可能会"，但是是的，对于今天的标准来说，大小只需要是O(n)。 - jk.

如果您调用它两次并且它确实是O(n)，那么它将是一个恒定的2倍因子，因此总体上仍然是O(n)。尽管如此，考虑这一点仍然很值得，因为人们经常假设大小始终为O(1)。 - jk.

@Peter Alexander 如果你要检查是否为空，请使用empty()而不是size :)。 - Passionate programmer

Answer 2

所以，第一个错误在这里：

std::list<int>::iterator it;

您在整数列表上定义了一个迭代器，然后使用它迭代双精度浮点数列表。此外，迭代器只能用于非常量列表。您需要一个常量操作符。您应该编写：

std::list<double>::const_iterator it;

最后，你忘记了返回值。

编辑：我没看见，但是你把列表作为引用传递，但是像指针一样使用。所以将所有的list->替换为list.

- PierreBdR

Answer 3

除了@PierreBdR的答案之外，您还应检查列表-＆gt;size()是否大于0，在此之前正确。

  avg /= list.size();

添加

 if (list.size()>0) 
    //avg code here.

或者记录下作为参数传递的列表不应为空。

   assert(list.size()>0)

- Tom

---

在编程中，更喜欢使用!list.empty()而不是list.size() > 0——前者具有常数复杂度，而后者具有线性复杂度。 - visitor

是的，我总是忘记那个...该死的Java。 - Tom

Answer 4

与其编辑以确保您的迭代器引用相同类型的列表，不如将代码编写为具有迭代器类型作为模板参数的通用算法。

我还注意到，对于std::list，您原始的代码和大多数发布的答案都存在严重的效率问题：给定列表的典型实现，它们会遍历列表两次以添加值，然后再次遍历以计算元素数量。理论上，list.size()可以在常数时间内运行而不需要遍历元素，但事实上很少这样（您可以在list::size()或list::splice中具有常数复杂度，但不能同时拥有）。

我会像这样编写代码：

template <class fwdit> 
typename fwdit::value_type arithmetic_mean(fwdit begin, fwdit end) { 

    typedef typename fwdit::value_type res_type;

    res_type sum = res_type();
    size_t count = 0;

    for (fwdit pos = begin; pos!= end; ++pos) { 
        sum += *pos;
        ++count;
    }
    return sum/count;
}

这段代码通用性很强，因此如果你发现使用std::list不太合适，需要改为使用std::vector时，不需做任何修改即可继续使用。同样，如果你需要对一些int进行算术平均而非double，也可以轻松处理（无需修改代码）。第三，即使像上面所提到的那样，你所使用的库的st::list::size()实现是线性的，仍然只需遍历一次链表，因此它的速度通常会比初始代码（已经在工作中）快两倍左右。

当然，缓存会对其产生影响 - 当你对一个小链表求平均值时，第一次遍历将整个链表拉入缓存，因此第二次遍历通常要快得多（因此消除第二次遍历并不能节省太多时间）。

- Jerry Coffin

---

这将无法按预期工作。例如，对于vector<int>返回类型为int，但平均值的类型为double，结果将被截断。 - 4pie0

因为你将其命名为“算术平均数”，但这样做不会保留算术平均数的属性。特别是：如果数字x_1，...，x_n的平均值为x，则（x_1-x）+... +（x_n-x）= 0。http://en.wikipedia.org/wiki/Arithmetic_mean - 4pie0

@piotruś：抱歉，但你的话并不太有意义。整数运算是整数运算，浮点数运算是浮点数运算。你不应该期望整数因为某种原因就变成浮点数。如果你想要像PHP那样在一瞬间无意义地转换类型...那就用PHP吧。 - Jerry Coffin

即使您似乎期望这样做，提供您的第二个解决方案：std::accumulate(v.begin(), v.end(), 0.0) / v.size(); 是的，您应该期望算术平均值返回正确的结果。浮点数或整数运算与此无关。 - 4pie0

IMO函数写错了。它返回了一些东西，但绝对不是平均值，也不是我所知道的任何统计数据。你应该警告它的行为并更改它的名称或更改它的行为。 - 4pie0

Answer 5

你传入了一个 std::list<double>，但是你创建了一个 std::list<int> 的迭代器？而且你的原型也接受一个 std::list<int>。

- Ron Warholic

---

这些应该是注释，不应该放在代码里面吗？ - Quartz

Answer 6

正如Maurits Rijk所说，您尚未返回平均值。此外，它正在编译哪些错误？

- batbrat

---

如果想要注释，请使用注释！ - Quartz

Answer 7

由于list是引用而不是指针，因此您不需要->解引用运算符，而只需要.运算符，即it = list.begin()等等。

另外，正如其他人指出的那样，列表及其迭代器的模板类型参数都需要匹配：要么是<int>，要么是<double>。看起来该函数最初是编写为接受double列表的。

- Derrick Turk

Answer 8

作为自己的练习，一旦你让它工作起来，你应该将其转换为for_each。从长远来看，这样更容易理解。

--编辑-- Accumulate更好，因为它适用于二进制数值运算。

- Hassan Syed

---

for_each 几乎从来不是一个好的算法选择，这里累加会更好。 - jk.

Answer 9

对于更复杂的对象，可以使用二元运算符来累加，如下所示：

auto accObjectValueA = [](double acc, const MyObject& object){ return acc + object.getValueA(); };
double sumValueA = std::accumulate(listOfObjects.begin(), listOfObjects.end(), 0, accObjectValueA );
double averageValueA = sumValueA  / listOfObjects.size();

- user2019716