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.

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.
    – s_hewitt
    Nov 23, 2009 at 18:08

9 Answers 9

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 :)

3
  • 2
    +1 for the USING suggestion. msdn.microsoft.com/en-us/library/yh598w02.aspx
    – Mike Atlas
    Nov 19, 2009 at 23:25
  • 2
    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.
    – J D OConal
    Nov 19, 2009 at 23:26
  • foreach (string line in File.ReadAllLines(fileName)) { ... } Nov 23, 2009 at 18:04
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.

5
  • So far the only person to answer his "main" question directly :)
    – Mike Atlas
    Nov 19, 2009 at 23:57
  • 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.
    – Alex
    Nov 20, 2009 at 1:58
  • 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.".
    – RCIX
    Nov 20, 2009 at 3:16
  • The FileStreams returned by File.Open* are already buffered, so you don't need to wrap them in a BufferedStream. Nov 23, 2009 at 18:00
  • 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.
    – Andy
    Dec 4, 2009 at 10:07
6

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

7
  • 2
    You are correct, of course, but in my experience, a lot of universities require an unnecessary level of commenting.
    – J D OConal
    Nov 19, 2009 at 23:23
  • 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). Nov 19, 2009 at 23:25
  • Yes, I personally agree with Mike's link, but my professor requires an enormous amount of commenting in our assignments.
    – Alex
    Nov 19, 2009 at 23:26
  • 3
    In a homework assignment like this, the how is the why. But in general you are correct.
    – ℍ ℍ
    Nov 19, 2009 at 23:30
  • 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.
    – Mike Atlas
    Nov 19, 2009 at 23:31
6

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

0
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

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

3
  • 1
    I haven't checked, but I think you need to declare line outside of the loop. Nov 19, 2009 at 23:37
  • Ugh, why do i always get those flipped? answer not question!
    – RCIX
    Nov 20, 2009 at 3:15
  • 1
    foreach (string line in File.ReadAllLines(fileName)) { ... } Nov 23, 2009 at 18:04
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;
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)));
}
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.

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.