Passing an argument to a method in C#

Question

So, this program functions as it should, but I don't think the code is very "clean," so I'm looking for suggestion. Two of the big issues I have:

1. For the method public double temperatureInFahrenheit, I want to pass the argument celsiusTemperature to the function instead of having to redeclare the variable, convert it from the text to the double, etc. Whenever I attempt to try that I get an error in the MessageBox.Show when I try to call the function myAirport.temperatureInFahrenheit (I don't specifically remember what the error is, so I'll have to recode if that's needed). Any thoughts on what I can do to make this work?

2. The MessageBox.Show in the public partial class Form1 seems like messy code to me. What I'd like to do is write a method in internal class Airport, where the necessary arguments are passed to the method, and then just do something like MessageBox.Show(myAirport.message()), but I'm assuming if I tried that I'd get the same error as I'm getting for 1. Any thoughts?

Again, the code is completely functional and works and meets the required specifications, but I don't like just having functional code, I like having functional code that's "pretty." Note: I don't have comments for any of the methods, variables, etc. I'm putting those in right now, but thought I'd try and get some feedback first.

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
    }

    private void button1_Click(object sender, EventArgs e)
    {
        string airportName;
        double celsiusTemperature, elevation;


        if (String.IsNullOrEmpty(txtAirport.Text)) {
            MessageBox.Show("You did not enter a name for the airport.  Please enter a name for the airport.");
            return;
        } else
        {
            airportName = Convert.ToString(txtAirport.Text);
        }
        if (Double.TryParse(txtTemperature.Text, out celsiusTemperature))
        {
            if (celsiusTemperature > 50 || celsiusTemperature < -50)
            {
                MessageBox.Show("The value you entered for temperature is outside the acceptable range.  Please reenter the information");
                Application.Restart();
            }
        }
        else
        {
            MessageBox.Show("You did not enter a numeric value for the temperature.  Please enter a valid, i.e. numeric, value for the temperature.");
            return;
        }
        if (Double.TryParse(txtElevation.Text, out elevation))
        {
            if (elevation > 12000 || elevation < -300)
            {
                MessageBox.Show("The value you entered for elevation is outside the acceptable range.  Please reenter the information.");
                Application.Restart();
            }
        }
        else
        {
            MessageBox.Show("You did not enter a numeric value for the elevation.  Please enter a valid, i.e. numeric, value for the elevation.");
            return;
        }

        Airport myAirport = new Airport(airportName, celsiusTemperature, elevation);

        MessageBox.Show("The airport name is: " + myAirport.airportName(airportName) + Environment.NewLine + "The Celsius temperature is: " + myAirport.celsiusTemperature(celsiusTemperature)
            + Environment.NewLine + "The Fahrenheit temperature is: " + myAirport.temperatureInFahrenheit(celsiusTemperature) + Environment.NewLine + "The elevation is: " + myAirport.elevation(elevation));

    }

    private void button2_Click(object sender, EventArgs e)
    {
        Close();
    }

}

internal class Airport
{
    private string airportName1;
    private double celsiusTemperature1;
    private double elevation1;

    public Airport(string airportName1, double celsiusTemperature1, double elevation1)
    {
        this.airportName1 = airportName1;
        this.celsiusTemperature1 = celsiusTemperature1;
        this.elevation1 = elevation1;
    }

    public string airportName(string airportName1)
    {
        return airportName1;
    }

    public double celsiusTemperature(double celsiusTemperature1)
    {
       return celsiusTemperature1;
    }

    public double elevation(double elevation1)
    {
        return elevation1;
    }

    public double temperatureInFahrenheit(double celsiusTemperature1)
    {

        double fahrenheitTemperature = 0;

        fahrenheitTemperature = celsiusTemperature1 * (1.8) + 32;

        return fahrenheitTemperature;
    }

}
}

Answer 1

There are many ways to separate the different requirements, so i try to give the idea.

First, a simple airport data class with properties and inside validation (That means invalid instances are possible):

internal class Airport
{
    private string _airportName;
    private double _celsiusTemperature;
    private double _elevation;

    public Airport(string airportName, double celsiusTemperature, double elevation)
    {
        this._airportName = airportName;
        this._celsiusTemperature = celsiusTemperature;
        this._elevation = elevation;
    }

    public string AirportName
    {
        get
        {
            return _airportName;
        }
        set
        {
            _airportName = value;
        }
    }

    public double CelsiusTemperature
    {
        get
        {
            return _celsiusTemperature;
        }
        set
        {
            _celsiusTemperature = value;
        }
    }

    public double Elevation
    {
        get
        {
            return _elevation;
        }
        set
        {
            _elevation = value;
        }
    }

    public double TemperatureInFahrenheit
    {
        get
        {
            return _celsiusTemperature * (1.8) + 32.0;
        }
        set
        {
            if (value != 32.0)
            {
                _celsiusTemperature = (value - 32.0) / (1.8);
            }
            else
            {
                _celsiusTemperature = 0.0;
            }
        }
    }

    public bool IsValid(out string errorMessage)
    {
        bool result = false;

        bool ok = true;
        errorMessage = "";

        if (String.IsNullOrEmpty(_airportName))
        {
            ok = false;
            errorMessage = "You did not enter a name for the airport.";
        }

        if (_celsiusTemperature > 50 || _celsiusTemperature < -50)
        {
            ok = false;
            errorMessage = "The value you entered for temperature is outside the acceptable range.";
        }

        if (_elevation > 12000 || _elevation < -300)
        {
            ok = false;
            errorMessage = "The value you entered for elevation is outside the acceptable range.";
        }

        result = ok;

        return result;
    }
}

Note that Fahrenheit is a calculated value based on Celsius. It could be vice versa.

Also note the validation. The airport class is accepting all values, only the type of the fields is defined. It does the semantic check (business logic).

Now, how to use it:

private void button1_Click(object sender, EventArgs e)
{
    string airportName;
    double celsiusTemperature;
    double elevation;

    // Get data from controls and do syntactic checks
    bool ok = true;
    airportName = txtAirport.Text;

    ok = Double.TryParse(txtTemperature.Text, out celsiusTemperature);
    if (!ok)
    {
        // Error
        MessageBox.Show("The value you entered for temperature is not a number!", "Error");
    }

    ok = Double.TryParse(txtElevation.Text, out elevation);
    if (!ok)
    {
        // Error
        MessageBox.Show("The value you entered for elevation is not a number!", "Error");
    }


    if (ok)
    {
        // Create the instance of the data class and do semantic checks
        Airport myAirport = new Airport(airportName, celsiusTemperature, elevation);
        string errorMessage;
        if (!myAirport.IsValid(out errorMessage))
        {
            // Error
            MessageBox.Show(errorMessage + " Please reenter the information", "Error");
        }
        else
        {
            // Ok, data is valid. Continue normal work...
            MessageBox.Show("The airport name is: " + myAirport.AirportName + Environment.NewLine +
                            "The Celsius temperature is: " + myAirport.CelsiusTemperature + Environment.NewLine +
                            "The Fahrenheit temperature is: " + myAirport.TemperatureInFahrenheit + Environment.NewLine +
                            "The elevation is: " + myAirport.Elevation);
        }
    }

You get the data from controls and do syntactic checks. If everthing is allright, you let the class do the semantic checks itself. Here it will give a string as message details, but many otehr ways are passible.

So, in the end, if a syntatic error occurs, the user gets a message and can continue. If a semantic error occurs, the user gets a message and can continue. If everything is ok, you can operate an valid data.

Hope this helps...

Answer 2

I think, you should refactor your code in the following ways:

• Airport seems to be a data class. So it should not know about the TextBox's of Form1. So, try to separate them.

• For easy access, you should think of properties (or getter/setter methods) for your fields in the airport class.

• You are checking the data when you are reading it out of the airport class. What if you are never reading it or using it inside before? So, you should separate this.

• If you estimate an error, you are restarting the complete application. That is normaly not a good deal to the user. So try to show the error and let the user do some corrections. And check again, and so on...

• If Airport is data class, then you can have to properties for Celsius and Fahrenheit. No matter what representation you have internaly.

I would like to give these hints without code first, so you can think and try for yourself before looking at a complete solution. Then, if you get stuck somewhere, i (and others) will give more concrete tips.

So, good luck for you...

Answer 3

Here is an example of how you could simplify the Airport class:

public class Program
{
    static void Main(string[] args)
    {
        var airport = new Airport { AirportName = "JFK", Temperature = 28.5 };

        Console.WriteLine(airport.ToString());
    }
}

public class Airport
{
    private string _airportName;

    private double _temperatureInCelsius;

    private double _temperatureInFahrenheit;

    public string AirportName
    {
        get
        {
            return _airportName;
        }
        set
        {
            if (string.IsNullOrWhiteSpace(value))
            {
                throw new Exception("You did not enter a name for the airport.  Please reenter the information.");
            }

            _airportName = value;
        }
    }

    public double Temperature
    {
        get
        {
            return _temperatureInCelsius;
        }
        set
        {
            if (value > 50 || value < -50)
            {
                throw new Exception("The value you entered for temperature is outside the acceptable range.  Please reenter the information");
            }

            _temperatureInCelsius = value;
            _temperatureInFahrenheit = _temperatureInCelsius *(1.8) + 32;
        }
    }
    public override string ToString()
    {
        return string.Format(
            "The airport name is: {0}\r\nThe Celsius temperature is: {1}\r\nThe Fahrenheit temperature is: {2}", _airportName, _temperatureInCelsius, _temperatureInFahrenheit);
    }        
}

Notice that the AirportName setter validates the passed airport name (via value) and if it's not valid, then an exception is thrown.

I will leave the other property - elevation, as an excercise for you to finish.

Regards the Message.Show you can do this:

Message.Show(airport.ToString());

The ToString() method return a string that describes the airport.

My code is a suggestion, you don't have to use it exactly (i.e. you might not like throwing an exception from the setters. Instead you could validate the values before creating an Airport instance.), but hopefully it will guide you.