Simple temperature converter

Question

Below is my first attempt at programming a Celsius to Fahrenheit converter in C# winforms. I'm looking for tips and advice on improving my style and what I can do to make the code more efficent/proper, or is it good as is? I apologize ahead of time if i've forgotten any details and would be happy to provide more upon request.

namespace TemperatureConverter
{
  public partial class Form1 : Form
  {
    public Form1()
    {
        InitializeComponent();
        // Initalize the handler to allow use of only control/number characters.
        this.txtCelsius.KeyPress += new KeyPressEventHandler(txtCelsius_KeyPress);
        // Limits user's inpute to 5 characters max.
        txtCelsius.MaxLength = 5;
        // Prevents the user from resizing window.
        this.FormBorderStyle = FormBorderStyle.FixedDialog;
    }
    // Main function. Converts the input into Fahrenheit and outputs to the targeted label.
    private void btnConvert_Click(object sender, EventArgs e)
    {
        double C;
        double F;

        C = double.Parse(txtCelsius.Text);
        F = C * 9 / 5 + 32;
        lblFahrenheit.Text = F.ToString();
    }
    // Prevents user from entering anything other than Number and Control characters.
    private void txtCelsius_KeyPress(object sender, KeyPressEventArgs e)
    {
        if (!Char.IsControl(e.KeyChar) && !Char.IsNumber(e.KeyChar))
        {
            e.Handled = true;
        }
    }
  }
}

Answer 1

The main change is separation of logic and User Interface. Firstly the calculation methods should ideally not reside in UI methods like event handlers.

Secondly the calculations should really be in it's own class, (but this application is just small enough that it is not ENTIRELY necessary).

---

in programming we hate Magic Numbers. If you can, move any numbers littered around your code and name them something useful.

so:

As 9/5 + 32 is equal to 1.8 (+32) and that number represents the scale difference, if we declare that at the top of our code like

double FAHRENEHEIT_SCALE_DIFFERENCE = 1.8;  
double FREEZING_POINT_OF_WATER = 32;

Depending on how much you have covered thus far you should then move the actual conversion to it's own method.

(because if you come back to this code in 12-18months will you be able to remember what it does? What if it quadruples in size with more features?

So:

I would suggest something along the lines of

public double convertCelsiusToFahrenheit(double celsius)
{
  return celsius * FAHRENEHEIT_SCALE_DIFFERENCE + FREEZING_POINT_OF_WATER;
}

Next, you can tidy the button click.

private void btnConvert_Click(object sender, EventArgs e)
{
    double celsius = double.Parse(txtCelsius.Text);
    lblFahrenheit.Text = convertCelsiusToFahrenheit(celsius);
}

---

Finally the golden change is the separation of the logic from the interface.

Now that you have a simple method that is separate from the actual screen; you could move it, and all similar methods into it's own TemperatureConverter class, or something similar.

Answer 2

Here's how I would encapsulate the logic:

public interface ITemperature
{
    double Temperature
    {
        get;
    }
}

public interface ITemperatureConverter<in T, out U> where T: ITemperature where U: ITemperature
{
    U Convert(T temperature);
}

public abstract class TemperatureBase : ITemperature
{
    private readonly double temperature;

    protected TemperatureBase(double temperature)
    {
        this.temperature = temperature;
    }

    public double Temperature
    {
        get
        {
            return this.temperature;
        }
    }
}

public sealed class Fahrenheit : TemperatureBase
{
    public Fahrenheit(double temperature) : base(temperature)
    {
    }

    public static implicit operator Fahrenheit(double temperature)
    {
        return new Fahrenheit(temperature);
    }

    public override string ToString()
    {
        return this.Temperature + "F";
    }
}

public sealed class Celsius : TemperatureBase
{
    public Celsius(double temperature) : base(temperature)
    {
    }

    public static implicit operator Celsius(double temperature)
    {
        return new Celsius(temperature);
    }

    public override string ToString()
    {
        return this.Temperature + "C";
    }
}

public sealed class FahrenheitToCelsiusConverter : ITemperatureConverter<Fahrenheit, Celsius>
{
    public Celsius Convert(Fahrenheit temperature)
    {
        if (temperature == null)
        {
            throw new ArgumentNullException("temperature");
        }

        return new Celsius((5f / 9f) * (temperature.Temperature - 32f));
    }
}

public sealed class CelsiusToFahrenheitConverter : ITemperatureConverter<Celsius, Fahrenheit>
{
    public Fahrenheit Convert(Celsius temperature)
    {
        if (temperature == null)
        {
            throw new ArgumentNullException("temperature");
        }

        return new Fahrenheit(temperature.Temperature * (9f / 5f) + 32f);
    }
}

Your UI code then becomes:

private readonly CelsiusToFahrenheitConverter celsiusToFahrenheitConverter = new CelsiusToFahrenheitConverter();

private void btnConvert_Click(object sender, EventArgs e)
{
    lblFahrenheit.Text = celsiusToFahrenheitConverter.Convert(double.Parse(txtCelsius.Text));
}