дать отзыв об этой программе указателя

Question

Это относительно простая программа. Но я хочу получить некоторые отзывы о том, как я могу улучшить эту программу (если таковые имеются), например, ненужные заявления?

#include<iostream>
#include<fstream>
using namespace std;

double Average(double*,int);

int main()
{

    ifstream inFile("data2.txt");

    const int SIZE = 4;
    double *array = new double(SIZE);
    double *temp;
    temp = array;

    for (int i = 0; i < SIZE; i++)
    {
        inFile >> *array++;
    }
    cout << "Average is: " << Average(temp, SIZE) << endl;
}

double Average(double *pointer, int x)
{
    double sum = 0;

    for (int i = 0; i < x; i++)
    {
        sum += *pointer++;
    }
    return (sum/x);
}

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

Спасибо.

Answer 1

Вы неправильно инициализируете свой массив. Это утверждение:

double *array = new double(SIZE);

Выделяет один дубль и инициализирует его значением SIZE. Что вы должны сделать, это использовать распределение массива:

double *array = new double[SIZE]; 

Другая общая проблема заключается в том, что вы редко когда-либо хотите назначить динамически выделенную память для необработанного указателя. Если вы хотите использовать базовые типы вместо объектов более высокого уровня, таких как std::vector, вы всегда должны использовать умный указатель:

boost::scoped_array<double> array(new double[SIZE]);

Теперь массив будет автоматически освобожден независимо от того, как вы покидаете свою область (т. Е. Из вновь добавленного возврата или из исключения).

Answer 2

Исправлена ​​утечка памяти. т.е. удалить темп; Также проверьте, открыт ли файл / создан..etc

в идеале вы должны манипулировать / перемещаться по массиву, используя вашу временную переменную, а не * сам массив

Answer 3

Поскольку мы говорим о C ++, я бы предложил использовать контейнеры и алгоритмы STL.Я также обнаружил, что в большинстве случаев лучше использовать ссылки или умные указатели (например, boost :: shared_ptr) вместо необработанных указателей.В этом случае указатели вообще не нужны.

Вот как вы могли бы написать свою программу:

#include <fstream>
#include <vector>
#include <iostream>
#include <numeric>
#include <iterator>

using namespace std;

int main()
{
    ifstream f("doubles.txt");
    istream_iterator<double> start(f), end;
    vector<double> v(start, end);

    if (v.empty())
    {
        cout << "no data" << endl;
        return 0;
    }

    double res = accumulate(v.begin(), v.end(), 0.0);
    cout << "Average: " << res / v.size() << endl;
    return 0;
}

Answer 4

Если x равно 0, то Среднее сгенерирует ошибку деления на ноль.

Answer 5

Вот некоторые комментарии к обзору кода:

В основном ():

1. Изменить размер на "size_ т" вместо int
2. Почему РАЗМЕР прописные? (Может быть, авторское соглашение состоит в том, чтобы иметь константы в верхнем регистре, в этом случае это нормально.)
3. Объедините временную декларацию и присваивание в одну инструкцию как: double * temp = array;
4. Что если inFile недоступен или не может быть открыт для чтения?
5. Что, если у inFile количество предметов меньше, чем РАЗМЕР?
6. Измените переменную цикла i на size_t.
7. Удалить пустую строку перед объявлением inFile.
8. Вернуть некоторое число (например, 0) из main().
9. Исправьте распределение массива.

В среднем ():

1. Изменить второй аргумент усреднения на size_t.
2. Утверждение и / или защита для указателя, отличного от NULL
3. Утверждение и / или защита от деления на ноль.

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