0

Our professor wants us to fix the code which counts the amount of values in a data.txt file and computes their average. Here is the code:

#include <cstdlib>
#include <iostream>
#include <string>
#include <fstream>
#include <vector>

using namespace std;

int main(int argc, char *argv[])
{
  string s;
  ifstream f;
  int nItems;
  double * data;
  double sum=0;
  vector < double > data2;
  double item;

  cout <<"File name: ";
  cin >> s;  
  f.open (s.c_str() );

  while (! f.eof() )
  {
    f >> item;
    data2.push_back(item);  
  }

  for (int i =0; i < nItems; i++)
  {
    sum += data[i];  
  }    
  cout << "The average is " << sum/nItems <<".\n";


    cout << "Press the enter key to continue ...";
    cin.get();
    return EXIT_SUCCESS;
}

His instructions were:

Modify code worked on today so that the average of data in vector < double > data is computed properly. Right now the code just gives you a value which isn't the average.

I tried changing the nItems variable into 12 and that seemed to work, but the goal of the code is to determine nItems and use that to find the average, which I can't seem to figure out.

  • 1
    This program exhibits undefined behavior, by way of accessing uninitialized variable `sum`. We leave the question of where and how to initialize and what value to initialize it to as an exercise for the reader. – Igor Tandetnik Mar 03 '19 at 05:07
  • 1
    The instructions talk about a `vector`, but there is no such `vector` in this code. – Remy Lebeau Mar 03 '19 at 05:31

2 Answers2

0

You use data for which you haven't allocated any memory when you summarise That causes undefined behaviour. You've stored your values in data2. Remove variables you don't need. Also, using namespace std is considered bad practice.

#include <iostream>
#include <fstream>
#include <vector>
#include <string>
#include <numeric> // std::accumulate

int main(int, char**) {
    std::string filename;

    std::cout << "File name: ";
    std::cin >> filename;
    std::ifstream f(filename); // no need for .c_str()

    std::vector<double> data;
    double item;

    // instead of checking for eof, check if extraction succeeds:
    while(f >> item) {
        data.push_back(item);
    }

    // a standard way to sum all entries in a container:
    double sum = std::accumulate(data.begin(), data.end(), 0.);

    std::cout << "The sum is " << sum << "\n";

    // use the containers size() function. it returns the number of items:
    std::cout << "The average is " << (sum / data.size()) << "\n";
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
-2

Hey looks like you weren't reading the numbers in from the txt file. Here I look through the input file once just to count how many numbers there are so I can make the array. Then I look through it again to fill the array.

#include <cstdlib> 
#include <iostream> 
#include <fstream> 
// f here stands for find, fstream gives you files

using namespace std;

int main(int argc, char *argv[]) {
    string s; 
    ifstream f; 
    // input file stream
    int nItems; 
    double * data; 
    double sum=0;

    cout << "File name: "; 
    cin >> s; 
    f.open (s.c_str() );
    // s.c_str will return a char array equivalent of the string s
    nItems=0;
    int input =0;
    while (f >> input) {//first loop reading through file to count number of items
        nItems++;
    }
    f.close();
    data = new double[nItems]; //Make the array
    f.open (s.c_str() );//open file for second read
    int i=0;
    while (f >> input) {//Second loop through file fills array
        data[i] = input;
        i++;
    }
    f.close();

    for (int i = 0; i < nItems; i++) {
        sum += data[i];
    }

    cout << "The average is " << sum / nItems << ".\n"; 

    cout << endl; 
    system("pause"); 
    return 0; 
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Ryan W
  • 3
  • 2
  • "*looks like you weren't reading the numbers in from the txt file*" - yes, he is. But he also reads the item count from the file, too, which may or may not be correct, as we don't know what the file actually looks like. "*Here I look through the input file once just to count how many numbers there are so I can make the array. Then I look through it again to fill the array.*" - you don't need to close and reopen the file, you can seek the stream back to the beginning of the file. But, in any case, the instructions clearly say to use a `vector` and not a `double[]` – Remy Lebeau Mar 03 '19 at 05:36