Page 1 of 1

Help with a code of mine

Posted: 2008-05-27 01:27am
by WesFox13
Hey can someone help me with a C++ code of mine? I need to make a code that can
Determine the daily min and max values of: Temperature, Pressure and Relative Humidity, Determine the daily max Wind Speed,
and Determine average Pressure, Relative Humidity and Wind Direction when rainfall is accumulated (display to console with single place after decimal)

Here is what I have in my code so far:

Code: Select all

///
///
///
///
///
/// Program: Weather data processing
/// Created by: Wesley Montgomery
/// This program anlizes weather data accessed from a file and finds the average of Temperature, Humidity, and other facts.
///
///
///

#include <fstream>	
#include <iostream>
#include <string>
#include <cmath>
#include <stdlib.h>
#include <algorithm>  // for min() and max()
#include <sstream>
#include <iomanip>


using namespace std;

void read_file(
  ifstream& file,        // The open and ready-to-read csv data file
  string    targetdate,  // The date we are interested in (see note below)
  int       temp, // The index of the field we are interested in
  double&   min,         // Where to put the minimum of the indexed field
  double&   max        // Where to put the maximum
);

void read_filepres(
  ifstream& file,        // The open and ready-to-read csv data file
  string    targetdate,  // The date we are interested in (see note below)
  int       pressure, // The index of the field we are interested in
  double&   min,         // Where to put the minimum of the indexed field
  double&   max        // Where to put the maximum
);

void read_filehum(
  ifstream& file,        // The open and ready-to-read csv data file
  string    targetdate,  // The date we are interested in (see note below)
  int       humidity, // The index of the field we are interested in
  double&   min,         // Where to put the minimum of the indexed field
  double&   max        // Where to put the maximum
);

void read_filewindmax(
  ifstream& file,        // The open and ready-to-read csv data file
  string    targetdate,  // The date we are interested in (see note below)
  int       wind, // The index of the field we are interested in
  double&   max        // Where to put the maximum
);

void get_dates( ifstream& file, string dates[], int& size );

int main()
{ 
	int pressure, temp, windspeed, humidity, rainfall, min_temp, max_temp, min_pres, max_pres, min_humi, max_humi, wind_max;
	bool  done; // exit flag 
	char  selection; //Menu Selection
	string file; // Name of datafile
	ifstream weatherfile; //datafile stream


	// Initialize exit flag
	 done = false;

	while (!done) 
	{
		// Output menu list
		cout << "\n\n";
		cout << "************************\n\n";		
		cout << "  Weather Data Analysis: \n";		
		cout << "  1) Select Data File to be loaded\n";		
		cout << "  2) Daily Min/max of Temp,Pressure and humidity\n";		
		cout << "  3) Daily Max Wind Speed\n";		
		cout << "  4) Avg Pressure, Humidity, and Windspeed for rainy days\n";	
		cout << "  5) \n";
		cout << "  6) Exit Program\n";
		cout << "************************\n\n";		

		// Input data					
		cout << "Selection -> ";		
		cin >> selection;
		cout << "\n\n";

		// Process input
		if (selection=='1') 
		{
			cout << "Insert Data file:";
			cin >> file; 
			weatherfile.open (file.c_str(), ios::in);
			cout << "\n\n";
		}
		else if (selection=='2')
		{
			while (!weatherfile.eof())
			{
				string targetdate = "04/18/2008"; // whatever the target day is
              read_file( weatherfile, targetdate, temp, min, max);
              cout << targetdate << ": ";
              cout << "The Minimum Temperature is" << setw(10) << setprecision(2) << min_temp << "degrees." << endl;
              cout << "The Maximum Temperature is" << setw(10) << setprecision(2) << max_temp << "degrees." << endl;
			  read_filepres( weatherfile, targetdate, pressure, min, max);
			  cout << "The Minimum Pressure is" << setw(10) << setprecision(2) << min_pres << "millibars." << endl;
              cout << "The Maximum Pressure is" << setw(10) << setprecision(2) << max_pres << "millibars." << endl;
			  read_filehum( weatherfile, targetdate, humidity, min, max);
			  cout << "The Minimum Humidity is" << setw(10) << setprecision(2) << min_humi << endl;
              cout << "The Maximum Humidity is" << setw(10) << setprecision(2) << max_humi << endl;
			}
			weatherfile.clear();
            weatherfile.seekg( 0 );
		}
		
		else if (selection=='3')
		{
			while (!weatherfile.eof())
			{
				string targetdate = "04/18/2008"; // whatever the target day is
              read_filewindmax(weatherfile, targetdate, windspeed, max);
              cout << targetdate << ": ";
              cout << "The Maximum Wind Speed is" << setw(10) << setprecision(2) << wind_max << "miles per hour." << endl;
			}
			weatherfile.clear();
            weatherfile.seekg( 0 );
		}
		else if (selection=='4')
		{
			while (!weatherfile.eof())
			{
			if (rainfall > 0)
			{ 

			}
			}
			weatherfile.clear();
            weatherfile.seekg( 0 );
		}
		else if (selection=='5')
		{
			while (!weatherfile.eof())
			{

				
			}
			weatherfile.clear();
            weatherfile.seekg( 0 );
		}
		else if (selection=='6')
		{
			cout << "Goodbye. Thank you for using my program.\n\n";
			done = true;					// flag for exit
		}
		else
		{
			cout << "** Invalid Selection. **\n\n";
		}
		weatherfile.close();

	} // end of while loop

	return (0);
}
I try to compile this but this error appears 4 times error C2664:read_filewindmax (filepres, filehum, file) cannot convert parameter 4 from 'overloaded-function' to 'double &'.
Can someone help me with this?

Posted: 2008-05-27 01:38am
by EnsGabe
You sure do seem to hate return values.

Aside from using a variable 'max' which seems to have no definition, you'll run into trouble when you pass an integer into the double precision 'return slots' you've put into your functions.

I think you'll be better off using return values instead of passing things around through parameters passed by reference. For the function that return more than one variable, make a struct to wrap them.

Posted: 2008-05-27 03:01am
by WesFox13
Wait how would using return values work in this situation? I always thought they just closed off the program after it was finished.

Posted: 2008-05-27 03:16am
by bilateralrope
If you have a method something like:

Code: Select all

int method(){
     stuff
     return x;
}
Where x is an integer declared in the method or its header. When the method ends, the value of x is sent back to whatever called it. So you call it with something like:

Code: Select all

int y = method();
Then y whatever x was when it was returned.

Though I have to wonder why you never learned about return values before.

Posted: 2008-05-29 11:31am
by Glass Pearl Player
Ensgabe pointed out some critical clue: The given code apparently does not define anything called 'max'. The compiler, on the other hand, seems to know the identifier.

Code: Select all

using namespace std;
together with the compiler error and a previous comment:

Code: Select all

#include <algorithm>  // for min() and max()
seems to solve the problem:
<algorithm> defines IIRC template functions named min and max within the std namespace. You try to use them in place of a double, and the compiler rightfully complains. As far as I can tell, main() does not need <algorithm> at all. Whether the other functions do need it, we cannot and need not know. If they are in another source file, we can (and IMHO should) go without that include.

WRT return values: A return statement exits the function in which it is contained. If that function happens to be main(), the program itself exits. If not, then not.

Nitpicks (if you haven't already taken care about them):
a) Your main() seems to be looooong. Pull out some code and put it in separate functions. I would strongly consider doing that.
b) you are using a long long if else if else if... construct to act depending on the value of a char. Did you consider the use of a switch statement?
c) (goes with d, somewhat) By the time I have read all the way down to (selection=='5'), I might have forgotten what option 5 actually was. Use some #define statements or constants or an enumeration type instead of magic numbers to help readability.

Posted: 2008-05-29 12:48pm
by starslayer
Just a couple stylistic comments/questions:

-Instead of writing your braces like you do, you might be able to improve your readability a bit by compacting you code: while (blah){, etc.
-You don't need to return 0 in main; it will do so implicity.
-You could also eliminate a lot of whitespace and still keep your program readable. This might actually improve readability, if your if else stuff and the selection table were closer on the screen to each other.

I'll second Glass Pearl Player's suggestion of a switch statement here, because with the input you're getting, it would be very simple to write and understand.

He could put his code in separate functions, but I've written longer mains than that, and I've only really just started programming. He's only using each selection code once, so with judicious use of comments, I don't know that it would really improve readability all that much. If he were running this stuff multiple times, I would certainly recommend writing functions to do the work.

Posted: 2008-05-29 02:10pm
by EnsGabe
If I were doing this, my main would look something like this:

Code: Select all

main(){
   while(true){
        print_menu()
        selection = get_selection()
        switch(selection){
            a: do_a()
            b: do_b()
            c: do_c()
            ...
            exit: { print_exit(); return EXIT_SUCCESS }

       }
   }

}
Insert appropriate language constructs where needed.

If you want to get really tricky, and do_* all take the same arguments, you can use function pointers like this:

Code: Select all



function_type functions[NUM_FUNCTIONS];

void populate_function_table(){
   functions[0]=&do_0;
   functions[1]=&do_1;
   ....
}
main(){
   populate_function_table()
   while(true){
        print_menu()
        selection=get_selection()
        if(selection == exit) return 0
        functions[selection](<INSERT PARAMETERS HERE>);       
   } 

}
function_type would be a typedef for the function pointer, which is non-trivial to figure out.

Even more fun/tricky would be having populate_function_table be in a separate file that you would generate automatically from *another* program by parsing the one you give it and including it in your primary source file.

Posted: 2008-05-29 10:55pm
by Mad
starslayer wrote:-Instead of writing your braces like you do, you might be able to improve your readability a bit by compacting you code: while (blah){, etc.
I find it easier to visually match up braces when they are aligned. It's not as compact, but having the mismatched alignment just looks ugly to me. It's personal preference, though.
-You don't need to return 0 in main; it will do so implicity.
He should return EXIT_SUCCESS.
-You could also eliminate a lot of whitespace and still keep your program readable. This might actually improve readability, if your if else stuff and the selection table were closer on the screen to each other.
The whitespace looks fine. If it were removed, the code would be more difficult to scan through.
I'll second Glass Pearl Player's suggestion of a switch statement here, because with the input you're getting, it would be very simple to write and understand.
Yes, this is exactly what switch() was intended for.
He could put his code in separate functions, but I've written longer mains than that, and I've only really just started programming. He's only using each selection code once, so with judicious use of comments, I don't know that it would really improve readability all that much. If he were running this stuff multiple times, I would certainly recommend writing functions to do the work.
For a program this size, it doesn't really matter. But as things get bigger, it's definitely better to use more functions, even if the code is only called once. The reader can more easily see the overall picture, and can drill down into the details as needed.

Posted: 2008-05-30 06:06am
by Durandal
Your main() is way too long and declared incorrectly. It should be declared as

Code: Select all

int main(void)
{
...
}
if you're not taking command line arguments. And yes, you should return 0 explicitly, as well as codes for any other error conditions your program might encounter.

Rule of thumb (or at least, the rule I use) is, when writing a function, think about the very basic steps involved. Then make sure that the code in your function expresses only those steps, or as few steps outside that core set as possible.

So instead of having gigantic amounts of C++ I/O calls (which are ugly enough to look at) inline in main(), do that printing in a separate function. That way, a person looking at the code can just see the primary logic main() is responsible for. It's the difference between "Okay, it's printing a bunch of crap" and "Okay, it's calling printUsage() to print its usage instructions". Believe it or not, functions aren't just for reusable code.

Posted: 2008-05-30 07:25am
by Dooey Jo
If it's supposed to be C++, it should not be "main(void)".

Posted: 2008-05-30 05:36pm
by Braedley
Mad wrote:
starslayer wrote:-Instead of writing your braces like you do, you might be able to improve your readability a bit by compacting you code: while (blah){, etc.
I find it easier to visually match up braces when they are aligned. It's not as compact, but having the mismatched alignment just looks ugly to me. It's personal preference, though.
It's very important to be consistent though. Don't do it both ways in the same file (or the same project for that matter).

Also, as far as your code goes, I would envision something along these lines:

Code: Select all

#include <includes>

#define DONE 6
#define other defines

int menu_select();  //use this to display the menu and get the result in one step.  converted to int because that's what our defines are
//other prototypes

int main()
{
    //declare variables
    int selection;

    selection=menu_select();
    while(selection!=DONE)
    {
        switch(selection)
        {
            //use your previous defines, ints will be used here as an example
            case 1:
                do1();
                break;
            case 2:
                do2();
                break;
            //...
            default:
                //do nothing
        }
        selection=menu_select();
    }

    return 0;
}

int menu_select()
{
    int input;

    cout<<menu;  //use the defines when outputting your menu!!

    cin>>input;
    return input;
}
Some of you may be wondering why I call menu_select twice. This is purely so that you don't have to rely on the compiler to make the proper optimizations regarding exiting the program, although in this case, it's minor. I could have just as easily used a do-while loop with one call to menu_select at the top of the loop.