Critique and Optimize my coding?

[Elarionus]

For this project in my class, we were given a text file with the average weather values for our little town for the first month of 2016. The values were input in 31 rows in 3 columns, the first of which was the date, the second being the high, and the third being the low.

Our objective was to put the values of the .txt file into arrays and then write functions to calculate the highest temperature, the lowest temperature, the average high, and the average low.

My program works! It lists all of the values, and then it displays the required values that are pulled from the functions.

I was hoping to get suggestions as I am still very new in C++. What comments/organizataion could I add to make it easier to read the code itself? What could I add to make the program easier for a user to read and use after it is compiled?

Thank you in advance for your help. I'd love to get better, and I'm afraid of being spoonfed in classes to the point that I'll get into the professional world and be helpless (or higher level classes).

Here is my code.

  /* FirstName_LastName
 * February 27, 2018
 * Section 02
 * Lab 07
 */

//I included comments as to what each thing does, and to my understanding of it.

#include <iostream>

//This is needed for opening the txt file (though I am unsure exactly what it does).
#include <fstream>
using namespace std;

ifstream infile; //I don't know what this is for. It was in the project description.

//Function Prototypes
double HighestTemp(double highArray[], int size);
double LowestTemp(double lowArray[], int size);
double AverageHigh(double highArray[], int size);
double AverageLow(double lowArray[], int size);


int main()
{
	//This opens the file that I'm getting the weather data for.
	infile.open("weather.txt");
	int i;

	//These are the arrays.
	int dateArray[31];
	double highArray[31];
	double lowArray[31];

	if (infile.is_open())
	{
		for (int i = 0; i < 31; i++)
		{
			infile >> dateArray[i];
			infile >> highArray[i];
			infile >> lowArray[i];
		}


		cout << "Dates    Highs    Lows\n";
		for (int i = 0; i < 31; i++)
		{
			cout << dateArray[i] << "    ";

			cout << highArray[i] << "    ";

			cout << lowArray[i] << "    " << endl;
		}
	}

	//This closes the file, because the data is already seeded into the arrays. I no longer need it, I believe.s
	infile.close();

	double Max;
	Max = HighestTemp(highArray, 31);
	cout << "\nThe highest temperature in January, 2016 was: " << Max << endl;
	
	double Min;
	Min = LowestTemp(lowArray, 31);
	cout << "\nThe lowest temperature in January, 2016 was: " << Min << endl;

	double AvgHigh;
	AvgHigh = AverageHigh(highArray, 31);
	cout << "\nThe average of all high temperatures in January, 2016 was: " << AvgHigh << endl;

	double AvgLow;
	AvgLow = AverageLow(lowArray, 31);
	cout << "\nThe average of all low temperatures in January, 2016 was: " << AvgLow << endl;

	//This is to give my program a nice clean exit.
	cout << "\n\nPress enter to exit.";
	cin.ignore();

	return 0;
}

double HighestTemp(double highArray[], int size)
{
	double Max = 0;
	for (int i = 0; i < size; i++)
	{
		if (highArray[i] > Max)
		{
			Max = highArray[i];
		}
	}

	return Max;
}

double LowestTemp(double lowArray[], int size)
{
	double Min = 0;
	for (int i = 0; i < size; i++)
	{
		if (lowArray[i] < Min)
		{
			Min = lowArray[i];
		}
	}

	return Min;
}

double AverageHigh(double highArray[], int size)
{
	double sum = 0;
	double AvgHigh = 0;
	for (int i = 0; i < size; i++)
	{
		sum = highArray[i] + sum;
	}

	AvgHigh = sum / size;

	return AvgHigh;
}

double AverageLow(double lowArray[], int size)
{
	double sum = 0;
	double AvgLow = 0;
	for (int i = 0; i < size; i++)
	{
		sum = lowArray[i] + sum;
	}

	AvgLow = sum / size;

	return AvgLow;
}

[jonnin]

infile is global, which is bad practice, and its used in main (infile.open() ) etc. Its a variable that is capable of working with files on the disk drive. At the very least it should not be global. If you don't need it, eliminate it. If you do need it, try to understand what it does and where it needs to be. (Hint, you need it).

you should next try replacing all the arrays with vectors. This eliminates the awkward size parameter attached to all your functions, so that can also be eliminated in favor of vector.size().

sum = highArray[i] + sum; is correct but consider sum+= highArray[i] which is easier to read and more expected code style.

Average high and average low are redundant. You should just have an average routine that averages whatever is sent to it. If you look at them, the two function bodies are identical apart from the names of things.

Those are some simple cleanups to start with. There are more things you can do beyond these that I am not good enough to write yet.

[icy1]

Usually basic arrays as arguments in functions are passed as pointers along with their sizes. As you currently have them, inside each of your methods (HighestTemp, etc), a copy of the array is made each time one of those function calls are made, which is not what you want. Edit: I completely forgot that arrays decay into pointers already; you can probably disregard this advice; that is, func(int a[]) is apparently going to be equivalent to func(int* a). I'm just so used to seeing the second variant

If you instead had everything using std::vector , not only would you be able to eliminate all the 31's, but you could also make your functions take references to vectors, and so keeping only one copy.

For files of unknown size, you can do it this way:

std::string file_name = "weather.txt";
ifstream ifs(file_name);
if (!ifs.is_open()) 
{
  cerr << "Failed to open stream for " << file_name << "." << endl;
  return -1;
}

int date;
double high, low;
while (ifs >> date >> high >> low)
{
   // Append to each of the vectors or whatever...

}
ifs.close();