C ++ STL List подсчитать среднее

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 ++ header:

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

Скорее всего, он предназначен для вычисления среднего значения из списка, но он соответствует множеству ошибок. Я пытался найти решения в Интернете, но ничего не смог найти. Я был бы рад, если бы кто-нибудь мог мне помочь.

Обновление: Спасибо за ваши быстрые ответы. Принятый ответ решил все мои проблемы.

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, если это возможно в ваших случаях использования.

Answer 2

Итак, первая ошибка есть:

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

Вы определяете итератор в списке целых чисел и используете его для итерации по списку двойников.Кроме того, итератор может использоваться только в непостоянном списке.Вам нужен постоянный оператор.Вы должны написать:

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

Наконец, вы забыли вернуть значение.

edit: Я не видел, но вы передаете список как ссылку, но используйте его как указатель.Так что замените все list-> на list.

Answer 3

Как насчет использования накапливать ?

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() окажется линейной, она все равно будет проходить по списку только один раз, так что, скорее всего, она будет примерно в два раза быстрее (рабочей версии) исходного кода.

Конечно, кеширование может (будет) влиять на это - когда вы усредняете небольшой список, первый обход перетянет весь список в кеш, поэтому второй обход обычно будет намного быстрее (поэтому устраняется второй обход) не сэкономит столько времени).

Answer 5

В дополнение к ответу @PierreBdR, Вы также должны проверить, что list-> size () больше 0,

Прямо перед здесь

  avg /= list.size();

добавить

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

Или документ, подтверждающий, что список, полученный в качестве аргумента, не должен быть пустым.

   assert(list.size()>0)

Answer 6

Как упражнение для себя. Как только вы заработаете, вы должны преобразовать его в for_each . Это легче понять в долгосрочной перспективе.

- редактировать -

Накапливать лучше, поскольку оно предназначено для двоичных числовых операций.

Answer 7

Поскольку list является ссылкой, а не указателем, вам не нужен оператор разыменования ->, а просто оператор ., т.е. it = list.begin() и т. Д.

Кроме того, как отмечали все остальные, параметры типа шаблона для списка и его итератора должны совпадать: либо <int>, либо <double>. Похоже, что функция изначально была написана для принятия списка double с.

Answer 8

Как сказал Мориц Рийк, вы не вернулись в среднем. Кроме того, с какими ошибками он компилируется?

Answer 9

Вы передаете std::list<double>, но создаете std::list<int> итератор? И ваш прототип также принимает std::list<int>.