Среднее значение и режим векторного массива. Как сделать меньшее улучшение в функции

Question

Выполнение упражнения для поиска mean и mode списка чисел, введенных пользователем. Я написал программу, и она работает, но мне интересно, слишком ли велика моя функция calcMode для этой программы. Я только начал изучать функции, которые являются первой попыткой. Было бы лучше написать меньшие функции? и если да, то какие части я могу разделить? Я довольно новичок в C ++ и также ищу, могу ли я улучшить этот код. Могу ли я сделать какие-либо изменения, чтобы сделать этот пробег более эффективным?

#include<iostream>
#include<vector>
#include<algorithm>
using namespace std;

int calcMean(vector<int> numberList) 
{

    int originNumber = numberList[0];
    int nextNumber;
    int count = 0;
    int highestCount = 0;
    int mean = 0;

    for (unsigned int i = 0; i <= numberList.size() - 1; i++) 
    {
            nextNumber = numberList[i];
            if (nextNumber == originNumber) 
                count++;
            else 
            {
                cout << "The Number " << originNumber << " appears " << count << " times." << endl;
                count = 1;
                originNumber = nextNumber;
            }
        }

    if (count > highestCount)
    {
        highestCount = count;
        mean = originNumber;
    }
    cout << "The Number " << originNumber << " appears " << count << " times." << endl;     
    return mean;
}

int main() 
{
    vector<int> v;
    int userNumber;
    cout << "Please type a list of numbers so we can arrange them and find the mean: "<<endl;

    while (cin >> userNumber)  v.push_back(userNumber);

    sort(v.begin(), v.end());

    for (int x : v)    cout << x << " | ";

    cout << endl;
    cout<<calcMean(v)<<" is the mean"<<endl;
    return 0;
}

Answer 1

Одна вещь, на которую стоит обратить внимание, это копирование векторов, когда вам это не нужно.

Функция подписи

int calcMode(vector<int> numberList)

означает, что numberList будет скопирован.

int calcMode(const & vector<int> numberList)

позволит избежать копирования. Скотт Майер Эффективный C ++ говорит об этом.

Кроме того, вызов numberList вводит в заблуждение - это не list.

Есть пара моментов, о которых стоит знать в цикле for:

for (unsigned int i = 0; i <= numberList.size()-1; i++)

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

for (unsigned int i = 0, size=numberList.size(); i <= size-1; i++)

size можно найти один раз таким образом, а не каждый раз. Они могут даже изменить i++ на ++i. Здесь привыкли к потенциальным накладным расходам, поскольку постинкремент может включать дополнительное временное значение

Один вопрос - вы * уверены, что это дает правильный ответ? Сравнение nextNumber == originNumber смотрит на первое число для начала. Попробуйте с 1, 2, 2.

Один последний момент. Если это общая цель, что произойдет, если список пуст?

Answer 2

Было бы лучше написать меньшие функции?

• Да, вы можете сделать ту же работу, используя std::map<>;, что может быть очень подходящий способ подсчета повторения элементов массива.
• Во-вторых, было бы гораздо безопаснее узнать, каков размер массив. Поэтому я предлагаю следующее:

std::cout << "Enter the size of the array: " << std::endl; std::cin >> arraySize;

• В calcMode() вы можете легко найти ссылку, так что массив не будет скопирован в функцию.

Вот обновленный код с указанным выше способом, на который вы можете ссылаться:

#include <iostream>
#include <algorithm>
#include <map>

int calcMode(const std::map<int,int>& Map)
{
    int currentRepetition = 0;
    int mode = 0;

    for(const auto& number: Map)
    {
        std::cout << "The Number " << number.first << " appears " << number.second << " times." << std::endl;

        if(currentRepetition < number.second )
        {
            mode = number.first;  // the number
            currentRepetition = number.second; // the repetition of the that number
        }
    }
    return mode;
}

int main()
{
    int arraySize;
    int userNumber;
    std::map<int,int> Map;

    std::cout << "Enter the size of the array: " << std::endl;
    std::cin >> arraySize;

    std::cout << "Please type a list of numbers so we can arrange them and find the mean: " << std::endl;
    while (arraySize--)
    {
        std::cin >> userNumber;
        Map[userNumber]++;
    }

    std::cout << calcMode(Map)<<" is the mode" << std::endl;
    return 0;
}

Обновление: После публикации этого ответа я обнаружил, что вы отредактировали свою функцию с помощью mean вместо mode. Я действительно не получил это.

Относительно среднего значения и режима: Я рекомендую вам прочитать больше. Потому что в общем случае набор данных может иметь несколько режимов и только одно среднее значение.

Answer 3

Лично я бы не разбивал этот код на более мелкие блоки, только если бы я хотел повторно использовать некоторый код в других методах. Но только для этого метода он более читабелен, как этот.

Порядок исключения aroun O (n) для calc, что вполне нормально, если вы спросите меня