Help with a code of mine

GEC: Discuss gaming, computers and electronics and venture into the bizarre world of STGODs.

Moderator: Thanas

Post Reply
User avatar
WesFox13
Padawan Learner
Posts: 274
Joined: 2007-02-14 11:50am
Location: Sammamish, WA, USA
Contact:

Help with a code of mine

Post 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?
My Political Compass:
Economic Left/Right: -5.25
Social Libertarian/Authoritarian: -5.90

Designation: Libertarian Left (Social Democrat/Democratic Socialist)
Alignment: Chaotic-Good
User avatar
EnsGabe
Youngling
Posts: 54
Joined: 2006-07-10 09:49pm

Post 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.
The Monarch: "Anyone wanna explain to me why my coccoon is charred?"
24: "Because you told us to blow it up"
The Monarch: "And why it is sideways?"
21: "We were following orders! You can't yell at us for following orders."
24: "Or kill us for following orders."
User avatar
WesFox13
Padawan Learner
Posts: 274
Joined: 2007-02-14 11:50am
Location: Sammamish, WA, USA
Contact:

Post 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.
My Political Compass:
Economic Left/Right: -5.25
Social Libertarian/Authoritarian: -5.90

Designation: Libertarian Left (Social Democrat/Democratic Socialist)
Alignment: Chaotic-Good
bilateralrope
Sith Acolyte
Posts: 6180
Joined: 2005-06-25 06:50pm
Location: New Zealand

Post 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.
Glass Pearl Player
Youngling
Posts: 81
Joined: 2003-02-19 04:51am
Location: somewhat against establishment

Post 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.
"But in the end-"
"The end of what, son? There is no end, there's just the point where storytellers stop talking."

- OotS 763

I've always disliked the common apologist stance that a browser is stable and secure as long as you don't go to the wrong part of the Internet. It's like saying that your car is bulletproof unless you go somewhere where you might actually get shot at. - Darth Wong
User avatar
starslayer
Jedi Knight
Posts: 731
Joined: 2008-04-04 08:40pm
Location: Columbus, OH

Post 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.
User avatar
EnsGabe
Youngling
Posts: 54
Joined: 2006-07-10 09:49pm

Post 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.
The Monarch: "Anyone wanna explain to me why my coccoon is charred?"
24: "Because you told us to blow it up"
The Monarch: "And why it is sideways?"
21: "We were following orders! You can't yell at us for following orders."
24: "Or kill us for following orders."
User avatar
Mad
Jedi Council Member
Posts: 1923
Joined: 2002-07-04 01:32am
Location: North Carolina, USA
Contact:

Post 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.
Later...
User avatar
Durandal
Bile-Driven Hate Machine
Posts: 17927
Joined: 2002-07-03 06:26pm
Location: Silicon Valley, CA
Contact:

Post 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.
Damien Sorresso

"Ever see what them computa bitchez do to numbas? It ain't natural. Numbas ain't supposed to be code, they supposed to quantify shit."
- The Onion
User avatar
Dooey Jo
Sith Devotee
Posts: 3127
Joined: 2002-08-09 01:09pm
Location: The land beyond the forest; Sweden.
Contact:

Post by Dooey Jo »

If it's supposed to be C++, it should not be "main(void)".
Image
"Nippon ichi, bitches! Boing-boing."
Mai smote the demonic fires of heck...

Faker Ninjas invented ninjitsu
User avatar
Braedley
Jedi Council Member
Posts: 1716
Joined: 2005-03-22 03:28pm
Location: Ida Galaxy
Contact:

Post 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.
Image
My brother and sister-in-law: "Do you know where milk comes from?"
My niece: "Yeah, from the fridge!"
Post Reply