4

我必须更正一些 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);

它很可能意味着从列表中计算平均值，但它符合很多错误。我试图在网上搜索解决方案，但我找不到任何东西。如果有人可以帮助我，我会很高兴。

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

4

8 回答 8

34

一些事情：

  1. 你不返回任何东西。（添加return avg;）
  2. ->运算符用于指向对象的指针。你有一个列表的引用，所以你使用list.begin()而不是list->begin()（其他成员函数相同）
  3. 迭代器应该是 a const_iterator，而不是iterator。

在任何情况下，您都应该执行以下操作：

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

可以选择检查list.size() == 0在您的用例中是否可行。

于 2010-02-01T13:37:39.720 回答
14

所以，第一个错误是：

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

您在整数列表上定义一个迭代器，并使用它来迭代一个双精度列表。此外，迭代器只能用于非常量列表。您需要一个常量运算符。你应该写：

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

最后，您忘记了返回值。

编辑：我没看到，但是您将列表作为参考传递，但将其用作指针。所以替换所有list->的list.

于 2010-02-01T13:33:11.210 回答
2

除了@PierreBdR 答案，您还应该检查 list->size() 是否大于 0，

就在这之前

  avg /= list.size();

添加

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

或记录作为参数接收的列表不应为空。

   assert(list.size()>0)
于 2010-02-01T13:38:36.007 回答
2

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

我还要注意，对于std::list，您的原始代码和大多数发布的答案都有一个相当严重的效率问题：给定一个典型的 list 实现，他们遍历列表一次以将值相加，然后再做一次计算元素的数量。理论上，list.size() 可以在不遍历元素的情况下以恒定的时间运行，但实际上这种情况很少见（您可以对list::size()or具有恒定的复杂性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()恰好是线性的，这仍然只遍历列表一次，因此它的速度可能是原始代码（工作版本）的两倍左右。

当然，缓存可以（将）影响这一点——当你平均一个小列表时，第一次遍历会将整个列表拉入缓存，所以第二次遍历通常会快很多（所以消除第二次遍历不会节省尽可能多的时间）。

于 2010-02-01T16:04:05.330 回答
1

正如 Maurits Rijk 所说，您还没有返回 avg。另外，它编译时有什么错误？

于 2010-02-01T13:33:56.633 回答
1

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

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

于 2010-02-01T13:33:59.947 回答
1

作为自己的锻炼。一旦你让它工作，你应该把它转换成一个for_each。从长远来看，它更容易理解。

- 编辑 -

Accumulate更好，因为它适用于二进制数字运算。

于 2010-02-01T13:36:57.863 回答
1

你传入一个std::list<double>但你创建了一个std::list<int>迭代器？你的原型std::list<int>也需要一个。

于 2010-02-01T13:32:54.520 回答