donner des commentaires sur ce programme pointeur

Question

Ce programme est relativement simple. Mais je veux avoir un retour sur la façon dont je peux améliorer ce programme (le cas échéant), par exemple, des déclarations inutiles?

#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); 
} 

Les codes sont valides et le programme fonctionne correctement. Mais je veux juste entendre ce que vous en pensez, puisque la plupart d'entre vous ont plus d'expérience que moi (eh bien je ne suis qu'un étudiant de première année ... lol)

Merci.

Answer 1

Corrigez la fuite de mémoire. i.e delete temp; En outre, vérifiez si le fichier est ouvert/created..etc

idéalement, vous devez manipuler/traverse le tableau en utilisant votre variable temporaire au lieu d'utiliser * tableau lui-même

Answer 2

Si x est 0, moyenne génèrera une erreur de division par zéro.

Answer 3

Puisque nous parlons de C++, je suggère d'utiliser des conteneurs et des algorithmes STL. Je trouve également que dans la plupart des cas, il est préférable d'utiliser des références ou des pointeurs intelligents (par exemple boost :: shared_ptr) au lieu de pointeurs bruts. Dans ce cas, il n'y a pas besoin de pointeurs du tout.

Voici comment vous pouvez écrire votre programme:

#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

Vous n'êtes pas correctement initialisez votre tableau. Cette déclaration:

double *array = new double(SIZE); 

Alloue un double et l'initialise à la valeur SIZE. Ce que vous devez faire est en utilisant l'allocation de tableau:

double *array = new double[SIZE]; 

Un autre problème général est que vous voulez rarement affecter la mémoire allouée dynamiquement à un pointeur brut. Si vous voulez utiliser des types de base au lieu d'objets plus haut niveau tels que std::vector, vous devez toujours utiliser un pointeur intelligent:

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

Maintenant, le tableau sera automatiquement se libérer peu importe la façon dont vous laissez votre portée (c.-à partir d'un nouveau retour ajouté ou d'une exception).

Answer 5

Voici quelques commentaires "l'examen du code":

En main():

1. Modifier la taille à "size_ t" au lieu de int
2. Pourquoi TAILLE est en majuscules? (Peut-être la convention de l'auteur est d'avoir des constantes en majuscules, dans ce cas, il est très bien.)
3. Combine déclaration temporaire et l'affectation dans une déclaration: double * temp = array;
4. si inFile n'est pas disponible ou ne peut pas être ouvert a lire?
5. Et si inFile avez moins de SIZE nombre d'articles?
6. Modifiez la variable de boucle i en size_t.Supprimer la ligne vide avant la déclaration de inFile.
7. Renvoyer un certain nombre (par exemple 0) de main().
8. Corrigez l'allocation du tableau.

En moyenne():

1. Modifier le deuxième argument de moyenne à size_t.
2. Affirmation et/ou protection pour pointeur non-nul
3. Affirmation et/ou protection contre la division par zéro.

Accusé de réception: Certains points sont collectés à partir d'autres réponses. J'ai essayé de faire une liste complète aussi loin que possible.