7

I have done a homework assignment, here is the problem statement:

Your program should work as follows:

  1. Ask the user to give you a file name. Get the file name and save it.
  2. Open the file.
  3. From the file read a temperature and a wind speed. Both values should be stored in variables declared as double. The file is a text file. Each line of the file contains a temperature and a wind speed value.

  4. Calculate the wind chill factor using a programmer written method, and display the result in the form:

    For t = temperature from file and v = wind speed from file Wind chill index = calculated result degrees Fahrenheit.

    Show all numbers with two digits after the decimal point. (Remember-no magic numbers!)

  5. Repeat these steps until an end of file is encountered.

I have completed the assignment, my code is below, I was just wondering if there was any way to make it more efficient, or if there are some different and creative ways to accomplish this problem, I already turned this in and got 50/50, but I'm just curious as to how some of you advanced and skilled programmers would approach this problem.

using System; using System.IO; class Program { // declare constants to use in wind chill factor equation - no magic numbers const double FIRST_EQUATION_NUMBER = 35.74; const double SECOND_EQUATION_NUMBER = 0.6215; const double THIRD_EQUATION_NUMBER = 35.75; const double FOURTH_EQUATION_NUMBER = 0.4275; const double EQUATION_EXPONENT = 0.16; const int DEGREE_SYMBOL_NUMBER = 176; static void Main() { // declare and initialize some variables string filePath = ""; string line = ""; double temperature = 0.0; double windSpeed = 0.0; double windChillFactor = 0.0; char degreeSymbol = (char)DEGREE_SYMBOL_NUMBER; // ask user for a file path Console.Write("Please enter a valid file path: "); filePath = Console.ReadLine(); // create a new instance of the StreamReader class StreamReader windChillDoc = new StreamReader(@filePath); // start the read loop do { // read in a line and save it as a string variable line = windChillDoc.ReadLine(); // is resulting string empty? If not, continue execution if (line != null) { string[] values = line.Split(); temperature = double.Parse(values[0]); windSpeed = double.Parse(values[1]); windChillFactor = WindChillCalc(temperature, windSpeed); Console.WriteLine("\nFor a temperature {0:f2} F{1}", temperature, degreeSymbol); Console.WriteLine("and a wind velocity {0:f2}mph", windSpeed); Console.WriteLine("The wind chill factor = {0:f2}{1}\n", windChillFactor, degreeSymbol); } } while (line != null); windChillDoc.Close(); Console.WriteLine("\nReached the end of the file, press enter to exit this program"); Console.ReadLine(); }//End Main() /// <summary> /// The WindChillCalc Method /// Evaluates a wind chill factor at a given temperature and windspeed /// </summary> /// <param name="temperature">A given temperature</param> /// <param name="ws">A given windspeed</param> /// <returns>The calculated wind chill factor, as a double</returns> static double WindChillCalc(double temperature, double ws) { double wci = 0.0; wci = FIRST_EQUATION_NUMBER + (SECOND_EQUATION_NUMBER * temperature) - (THIRD_EQUATION_NUMBER * (Math.Pow(ws, EQUATION_EXPONENT))) + (FOURTH_EQUATION_NUMBER * temperature * (Math.Pow(ws, EQUATION_EXPONENT))); return wci; } }//End class Program

Feel free to tell me what you think of it.

Improve this question
1
  • 1
    When you have a function that returns a calculated result based on a set equation, it's nice to have the equation written in the summary of the function. Commented Nov 23, 2009 at 18:08

9 Answers 9

Reset to default
9

Your way looks good, but:

  • It would look nicer if you used PascalCase for the constants, as that's what coding conventions for c# use.
  • you should wrap the StreamReader in a using statement, so it gets properly disposed once you're done.
  • You should probably also wrap it in a try block (and a catch to properly handle the exception) to make sure that you don't get a FileNotFound exception.
  • It's probably a better idea to structure your while loop the following way:

while((line = windChillDoc.ReadLine()) != null) { ... } [Darn formatting won't work right!]

Other than that though, i wouldn't know as i'm not familiar with weather calculations :)

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

+1 for the USING suggestion. msdn.microsoft.com/en-us/library/yh598w02.aspx
I don't agree that it would look 'nicer', but you're right -- the conventions state that PascalCase should be used for constants. I kind of like the UPPERCASE_WITH_UNDERSCORES style, as it clearly differentiates comments and has been used for such a long time.
foreach (string line in File.ReadAllLines(fileName)) { ... }
7

You're not going to get much zippier than that for file IO in C#. Depending on the size of the data set it may be worth using a buffered reader, but for sufficiently small files, it's just not worth it. I'd leave it as-is.

Improve this answer

5 Comments

So far the only person to answer his "main" question directly :)
Thanks for the answer, your right of course, with a simple file io program like this, you can't get a whole lot more efficient. P.S Thank all of you for your suggestions, they were all helpful.
I would point out that he didn't ask this directly, he asked "I'm just curious as to how some of you advanced and skilled programmers would approach this problem.".
The FileStreams returned by File.Open* are already buffered, so you don't need to wrap them in a BufferedStream.
Agreed. If moving the stream position around, though, memory mapped files can give a performance boost (and they are part of .net 4). Sequential access performance is almost identical to filestream, though.
6

Most of your comments are extraneous. The code should tell you how...the comments should tell you why.

Improve this answer

7 Comments

You are correct, of course, but in my experience, a lot of universities require an unnecessary level of commenting.
It's true. My professors had an uncanny love for comments and often had us include them just for the sake of them being there. It's a pretty big waste of time if you ask me. Comments should only really be necessary if the implementation is not obvious (or you're creating a documented system).
Yes, I personally agree with Mike's link, but my professor requires an enormous amount of commenting in our assignments.
In a homework assignment like this, the how is the why. But in general you are correct.
Sucks, but whatya gonna do? At least you know how to get the points. Just as an aside, this is a pretty basic program. You're not going to get much more "efficient" file i/o unless you get better hardware.
|
6

Minor nitpick, but "WindChillCalc" should be "CalcWindChill" if you are are using English method names (the verb goes first).

Improve this answer

Comments

5 
string filePath = ""; ... filePath = Console.ReadLine();

Don't initialize with values that are never used; and keep declaration and initialization close together:

string filePath = Console.ReadLine();

using has been mentioned - but don't use @ unnecessarily:

new StreamReader(@filePath);

should be just:

new StreamReader(filePath);

Personally, I'd use LINQ for the line-reader, but that is just me ;-p

Improve this answer

Comments

4

Why the do/while? In your do you check for null. In your while you check for null. Why not just make it a while statement?

string line; while((line = windChillDoc.ReadLine()) != null) { //Logic }

EDIT: Fixed the compilation error. Funny thing was I had that originally. This Rich Text Box needs a compiler! :P

Improve this answer

3 Comments

I haven't checked, but I think you need to declare line outside of the loop.
Ugh, why do i always get those flipped? answer not question!
foreach (string line in File.ReadAllLines(fileName)) { ... }
3

Although not really relating to performance (the main question)

IMO:

const double FIRST_EQUATION_NUMBER = 35.74; const double SECOND_EQUATION_NUMBER = 0.6215; const double THIRD_EQUATION_NUMBER = 35.75; const double FOURTH_EQUATION_NUMBER = 0.4275; const double EQUATION_EXPONENT = 0.16;

isn't much better than a magic number. Looking at that I have no idea what FIRST_EQUATION_NUMBER is used for other than in an equation somewhere, and I can't tell that they are in the same equation or you have four equations which use different numbers? They could also be put into the actual method as thats the only place they are used.

I would change degreeSymbol to a const rather than working it from a const int later.

const char DEGREE_SYMBOL = (char)176;
Improve this answer

Comments

2

If you're getting marked on style etc. then there's a couple extremely minor things

  • Initializing doubles to 0.0 is redundant.
  • string.Empty is preferred instead of ""
  • Your windchill method can be changed to simply (though, during compilation, I think that wci will be optimized out - so it's functionally the same):

(changed formatting for SO readability)

static double WindChillCalc(double temperature, double ws) { return FIRST_EQUATION_NUMBER + (SECOND_EQUATION_NUMBER * temperature) - (THIRD_EQUATION_NUMBER * (Math.Pow(ws, EQUATION_EXPONENT))) + (FOURTH_EQUATION_NUMBER * temperature * (Math.Pow(ws, EQUATION_EXPONENT))); }
Improve this answer

Comments

0

In little academic programs like this, unless you do something really dumb, performance will not be an issue. A simple way to tell if performance is an issue is to ask the question: "Does it make me wait?"

If there were an enormous amount of input, I would ask who's providing the input and who's reading the output. That would tell me if I could do the I/O in binary rather than text, because as it is, by far the bulk of the processing will be in the conversion of text into numbers on input, and numbers to text on output, especially floating point.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.