Temperature conversion application

Question

I'm two months in a programming course and our instructor told us we should think about building real-world applications as to gain valuable experience. I got the idea of doing a simple temperature conversion app.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace TempratureProject
{
    class Program
    {
        static void Main(string[] args)
        {   
            //Ask for the temprature that the user wants to convert
            Console.WriteLine("What is the temprature you wish to convert?");
            var RInput = Console.ReadLine();
            double dInput = Convert.ToDouble(RInput);
            //Ask for what type of conversion
            Console.WriteLine("If you wish to convert Fahrenheit to Celsius, Press 1");
            Console.WriteLine("If you wish to convert Fahrenheit to Kelvin, Press 2");
            Console.WriteLine("If you wish to convert Celsius to Fahrenheit, Press 3");
            Console.WriteLine("If you wish to convert Celsius to Kelvin, Press 4");
            Console.WriteLine("If you wish to convert Kelvin to Fahrenheit, Press 5");
            Console.WriteLine("If you wish to convert Kelvin to Celsius, Press 6");
            //Set YInput equal to the choice made
            var YInput = Console.ReadLine();
              int iInput = Convert.ToInt32(YInput);
            //Use of a a series of If statements to call the methods
              if (iInput == 1)
              {
                  var aAnswer = FtoCelsius(dInput);

                  Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Celsius", dInput, aAnswer);
              }
              else if (iInput == 2)
              {
                  var bAnswer = FtoKelvin(dInput);

                  Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Kelvin", dInput, bAnswer);
              }
              else if (iInput == 3)
              {
                  var cAnswer = CtoFahenreit(dInput);

                  Console.WriteLine("The value of {0} degrees celcius is equall to {1} degrees Fahrenheit", dInput, cAnswer);
              }
              else if (iInput == 4)
              {
                  var dAnswer = CtoKelvin(dInput);

                  Console.WriteLine("The value of {0} degrees celsius is equall to {1} degrees Kelvin", dInput, dAnswer);
              }
              else if (iInput == 5)
              {
                  var eAnswer = KtoFahrenheit(dInput);

                  Console.WriteLine("The value of {0} degrees Kelvin is equall to {1} degrees Fahrenheit", dInput, eAnswer);
              }
            else
              {
                  var fAnswer = KtoCelsius(dInput);

                  Console.WriteLine("The Value of {0} degrees Kelvin is equall to {1} degrees Celsius", dInput, fAnswer);  
              }


        }
        //The various methods
        static double FtoKelvin(double CTempIn)
        {
            double fKel = (CTempIn - 32) * (5 / 9) + 273.15;

            return fKel;
        }
        static double FtoCelsius(double CTempIn)
        {
            Double fCels = (CTempIn - 32) * (5 / 9);

            return fCels;
        }
         static double CtoFahenreit(double CtempIn)
        {
            double dFahr = (1.8) * (CtempIn + 32);

            return dFahr;
        }
         static double CtoKelvin(double CTempIn)
        {
            double cKel = CTempIn + 273.15;

            return cKel;
        }
         static double KtoFahrenheit(double CTempIn)
         {
             double kFah = (CTempIn - 273.15) * (9 / 5) + 32;

             return kFah;
         }
         static double KtoCelsius(double CTempIn)
         {
             double KCel = CTempIn - 273.15;

             return KCel;
         }
    }
}

It is working so there is that but I would like to know how well this is written and how I could make it better.

Just my opinion, but it is pretty much what it should be. I might choose different names etc but that has nothing to do with your code. IF you want a suggestion then you can use this in your methods "return (CTempin - 273.15);" instead. Also from my experience make sure the value for 273.15C has enough precision for your purposes. I do not think defining a variable just to hold the answer for the subsequent return adds much in the way of understanding. — Enigma Maitreya, Mar 16, 2017 at 17:18
@EnigmaMaitreya, -273.15 °C definitely has enough precision for the absolute zero point, since it's defined that way! As for the stored precision, an IEEE double has about 15 significant digits of precision, which is way more than any engineering context will require. — ilkkachu, Mar 16, 2017 at 22:34
There is so much good stuff in these answers, I wish I could accept all of them. — Neil Meyer, Mar 17, 2017 at 12:12

πάντα ῥεῖ · Accepted Answer · 2017-03-17 09:41:47Z

UI experience

Your menue text is hard to read and not intuitively clear. I'd prefer to see something like that:

     Console.WriteLine(
@"Choose to convert temperature values, enter a key please:\n
1 - Fahrenheit to Celsius
2 - Fahrenheit to Kelvin
3 - Celsius to Fahrenheit
4 - Celsius to Kelvin
5 - Kelvin to Fahrenheit
6 - Kelvin to Celsius, Press 6
> ");

Also outputting the text that way would make the code more readable.

Taking input from console

Your statements

 var YInput = Console.ReadLine();
 int iInput = Convert.ToInt32(YInput);

can be simplified to

 int iInput = Convert.ToInt32(Console.ReadLine());

consider to catch exceptions or use a String and TryParse() to check for invalid input.¹

Rather use switch than if else cascades

You should replace

 if (iInput == 1) 
          {
              var aAnswer = FtoCelsius(dInput);

              Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Celsius", dInput, aAnswer);
          }
 else if (iInput == 2)
          {
              var bAnswer = FtoKelvin(dInput);

              Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Kelvin", dInput, bAnswer);
          }

with

switch(iInput) {
case 1:
    var aAnswer = FtoCelsius(dInput);
    Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Celsius", dInput, aAnswer);
    break;
case 2:
    var bAnswer = FtoKelvin(dInput);
    Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Kelvin", dInput, bAnswer);
    break;
}

That makes your code more readable and concise.

Write generic code, if it's prone to be extended

At least I'd try to make all of that more generic, using a

public delegate double ConversionDelegate(double x); 
Dictionary<Tuple<string,string>,ConversionDelegate) = {
    { {"Fahrenheit","Celsius"}, FtoCelsius } ,
    { {"Fahrenheit","Kelvin"}, FtoKelvin} ,
    // ...
    { {"Kelvin", "Celsius" }, KtoCelsius ,
};

dictionary pair. Where the tuple key serves the possible combinations of Fahrenheit to Celsius, Kelvin to Fahrenheit, etc. You can use the functions from Dictionary to match the key, and call the delegate and render the output accordingly.

The command line input should look like this then

> Enter temperature and <unit> <conversion unit>
> 39.5 celsius kelvin
> The value of 39.5 degrees celsius is equal to 312,65 degrees Kelvin

¹⁾_{BTW you rather want to deal with double values IMO.}

@t3chb0t Good catch. I'm going to completely ovehaul that with a generic approach as sketched out in my last paragraph. — πάντα ῥεῖ, Mar 16, 2017 at 17:48
Not a nit just an observation I might avoid delegate because of the beginner tag. — Enigma Maitreya, Mar 16, 2017 at 17:49
@EnigmaMaitreya What's actually wrong with delegate for beginners? — πάντα ῥεῖ, Mar 16, 2017 at 17:52

Matthew FitzGerald-Chamberlain · Accepted Answer · 2017-03-16 21:46:06Z

Two items I would have a look at in terms of the actual calculations.

Use Full Names in Methods

For clarity, and because the few characters are inconsequential, I'd prefer to see the full names of the units being used in the method names. This will improve readability and understandability of the code.

static double CelsiusToFahrenheit(double celsiusValue) {
    return (celsiusValue + 32) * 1.8;
}

Factor Out Constants

Several of the numbers used are constants which could be factored out, such as the difference between Kelvin and Celsius (273.15). This will prevent typos from causing calculation errors. Something like the following:

private const double KelvinToCelsiusDifference = 273.15;

static double KelvinToCelsius(double kelvinValue) {
    return kelvinValue + KelvinToCelsiusDifference;
}

Stephen Rauch · Accepted Answer · 2017-03-17 00:13:14Z

Fix the type errors in the calculations

The biggest issue with the code is that it's not performing the calculations correctly due to type errors.

For example, in

static double FtoKelvin(double CTempIn)
{
    double fKel = (CTempIn - 32) * (5 / 9) + 273.15;

    return fKel;
}

, (5 / 9) has both 5 and 9 as integers, so that fraction reduces to zero, effectively reducing the method to

static double FtoKelvin(double CTempIn)
{
    double fKel = 273.15;

    return fKel;
}

. One easy way to fix this would be to put a .0 after the integer values, e.g.

static double FtoKelvin(double CTempIn)
{
    double fKel = (CTempIn - 32.0) * (5.0 / 9.0) + 273.15;

    return fKel;
}

Remove the options

Since you're just doing a few cheap-and-easy conversions, you may as well do all of the possible conversions each time and show the user a single line that gives them each.

The rationale is that there's no reason to hide the other unit conversions from the user; it's easier for them to just pick out the value that they're interested in rather than sort through a menu of options to ask for it. Also, they may want a comparison of several, in which case it's even better for them.

Jamal · Accepted Answer · 2017-08-06 14:35:38Z

You better get rid of all the comments! Comments are evil, seriously!

For example:

//Ask for the temprature that the user wants to convert
Console.WriteLine("What is the temprature you wish to convert?");

By just looking at the code I can understand that you're asking for an input, so no comment needed here. The same goes for other comments.

OK, let's talk about function usage. I know most of the times when people teaching programming, they say function is for reusing a piece of code. Yes, that's true, but most of the time, professionals use function to increase the readability. As a basic guideline, whenever you need to write a comment to explain yourself, consider using a function instead.

For example:

class Program
{
  double temperature;

  static void Main(string[] args)
  {
    getUserRequest();
    calculateTheConversion();
    showTheResult();
  }

  void getUserRequest()
  {
    askForTemperature();
    askForConversionType();
  }

  void askForTemperature()
  {
    Console.WriteLine("What is the temprature you wish to convert?");
    temperature = Convert.ToDouble(Console.ReadLine());
  }

  void askForConversionType()
  {
    //other code here...
  }

  // and the rest of methods go here...
}

By writing your code this way, the reader (which can be yourself after some months) can quickly understand what's going on and if he or she wants see more detail, can go inside each function/method and read more.

Well, comments are not evil per se, but pointless comments iterating what the code does (and not why) definitely are. Totally agree on the rest, though. I'll take a nicely named function (preferably with a docstring describing its usage) over commented blocks of code any day. — Graipher, Mar 17, 2017 at 11:32

levent · Accepted Answer · 2017-03-17 12:00:07Z

you can predefined your operations.

class Program
{
    class Operation
    {
        internal string Key;
        internal string Message;
        internal string ResultMessage;
        internal Func<double, double> Function;
    }
    static List<Operation> operations = new List<Operation> { 
        new Operation {
            Key = "1",
            Message = "If you wish to convert Fahrenheit to Celsius, Press 1",
            ResultMessage = "The value of {0} degrees Fahrenheit is equall to {1} degrees Celsius",
            Function = (CTempIn) => (CTempIn - 32) * (5 / 9)
        },
        new Operation{
            Key = "2",
            Message = "If you wish to convert Fahrenheit to Kelvin, Press 2",
            ResultMessage = "The value of {0} degrees Fahrenheit is equall to {1} degrees Kelvin",
            Function = (CTempIn) => (CTempIn - 32) * (5 / 9) + 273.15
        },
        new Operation
        {
            Key = "3",
            Message = "If you wish to convert Celsius to Fahrenheit, Press 3",
            ResultMessage = "The value of {0} degrees celcius is equall to {1} degrees Fahrenheit",
            Function = (CTempIn) => (1.8) * (CTempIn + 32)
        },
        new Operation
        {
            Key = "4",
            Message = "If you wish to convert Celsius to Kelvin, Press 4",
            ResultMessage = "The value of {0} degrees celsius is equall to {1} degrees Kelvin",
            Function = (CTempIn) => CTempIn + 273.15
        },
        new Operation
        {
            Key = "5",
            Message = "If you wish to convert Kelvin to Fahrenheit, Press 5",
            ResultMessage = "The value of {0} degrees Kelvin is equall to {1} degrees Fahrenheit",
            Function = (CTempIn) => (CTempIn - 273.15) * (9 / 5) + 32
        },
        new Operation
        {
            Key = "6",
            Message = "If you wish to convert Kelvin to Celsius, Press 6",
            ResultMessage = "The Value of {0} degrees Kelvin is equall to {1} degrees Celsius",
            Function = (CTempIn) => CTempIn - 273.15
        }
    };
    static void Main(string[] args)
    {

        //Ask for the temprature that the user wants to convert
        Console.WriteLine("What is the temprature you wish to convert?");
        var RInput = Console.ReadLine();
        double dInput = Convert.ToDouble(RInput);
        //Ask for what type of conversion
        foreach (var item in operations)
        {
            Console.WriteLine(item.Message);
        }
        var YInput = Console.ReadLine();

        var operation = operations.Where(o => o.Key.Equals(YInput)).SingleOrDefault();
        if(operation == null)
        {
            Console.WriteLine("invalid input");
            Console.ReadLine();
            return;
        }
        Console.WriteLine(operation.ResultMessage,dInput, operation.Function(dInput));
        Console.ReadLine();

    }

}

UPDATE:

it can be more generic

class Program
{
    class Operation
    {
        internal string Key;
        internal string From;
        internal string To;
        internal Func<double, double> Function;
    }
    static string Message = "If you wish to convert {0} to {1}, Press {2}";
    static string ResultMessage ="The value of {0} degrees {2} is equall to {1} degrees {3}";
    static List<Operation> operations = new List<Operation> { 
        new Operation { Key = "1", From="Fahrenheit", To="Celsius", Function = (CTempIn) => (CTempIn - 32) * (5 / 9) },
        new Operation { Key = "2", From="Fahrenheit", To="Kelvin", Function = (CTempIn) => (CTempIn - 32) * (5 / 9) + 273.15 },
        new Operation { Key = "3", From="Celsius", To="Fahrenheit", Function = (CTempIn) => (1.8) * (CTempIn + 32) },
        new Operation { Key = "4", From="Celsius", To="Kelvin", Function = (CTempIn) => CTempIn + 273.15 },
        new Operation { Key = "5", From="Kelvin", To="Fahrenheit", Function = (CTempIn) => (CTempIn - 273.15) * (9 / 5) + 32 },
        new Operation { Key = "6", From="Kelvin", To="Celsius", Function = (CTempIn) => CTempIn - 273.15 }
    };
    static void Main(string[] args)
    {

        //Ask for the temprature that the user wants to convert
        Console.WriteLine("What is the temprature you wish to convert?");
        var RInput = Console.ReadLine();
        double dInput = Convert.ToDouble(RInput);
        //Ask for what type of conversion
        foreach (var item in operations)
        {
            Console.WriteLine(Message,item.From,item.To,item.Key);
        }
        var YInput = Console.ReadLine();

        var operation = operations.Where(o => o.Key.Equals(YInput)).SingleOrDefault();
        if(operation == null)
        {
            Console.WriteLine("invalid input");
            Console.ReadLine();
            return;
        }
        Console.WriteLine(ResultMessage,dInput, operation.Function(dInput),operation.From,operation.To);
        Console.ReadLine();

    }

}

Apart from the fields that should be properties this is of course the best solution ;-] — t3chb0t, Mar 17, 2017 at 12:09
If only you were not so late to the party, I would have probably accepted this. — Neil Meyer, Mar 17, 2017 at 12:13
The code above gives incorrect answers; 5/9 == 0and 9/5 == 1. Gotta make them floating-point types. — Nat, Mar 17, 2017 at 13:03
The List should really be a Dictionary since you're just using it to look up a .Key. — Nat, Mar 17, 2017 at 13:07
The only issue I focus on is the approach. Functions may vary. — levent, Mar 17, 2017 at 13:15

el.pescado - нет войне · Accepted Answer · 2017-03-17 14:18:51Z

Variable prefixes

You seem to use some kind of Hungarian notation, judging from these:

double dInput = Convert.ToDouble(RInput);
int iInput = Convert.ToInt32(YInput);
var aAnswer = FtoCelsius(dInput);

But it's totally unclear to me what those prefixes mean. I'd remove those prefixes, especially that nowadays most people agree against Hungarian notation.

As an exception to this rule you could prefix variables that are supposed to hold values in certain unit. That would make spotting errors easier:

double someVar = inputFromUser; // error, but hard to spot
double cSomeVar = fInputFromUser; // error, easy to spot
double cSomeVar = FtoCelsius(fInputFromUser); // ok

Boxed types and type inferrence

You interchange boxed types (eg. Double) with primitive types (double).

double fKel = (CTempIn - 32) * (5 / 9) + 273.15;
Double fCels = (CTempIn - 32) * (5 / 9);

They're not the same, you should use primitive types when possible. Moreover, you use type-inferred variables:

var RInput = Console.ReadLine();
double dInput = Convert.ToDouble(RInput);

That's good, but you should try to be consistent - either use var everywhere or spell out types everywhere.

Style guidelines

According to C# style guidelines, variable names should start with lower case letter.

Unused imports

You seem to have some unnecessary/unused imports

Disclaimer: It's been a while since I last programmed in C#.

paparazzo · Accepted Answer · 2017-03-16 17:37:05Z

1

Need to check for valid input her and range

int iInput = Convert.ToInt32(YInput);

What if YInput = "f" ?
Or 0 or 100

Same thing with RInput. What if they enter a letter?

answered Mar 16, 2017 at 17:37

paparazzo

5,9463 gold badges18 silver badges40 bronze badges

Add a comment |

ilkkachu · Accepted Answer · 2017-03-16 22:45:43Z

Console.WriteLine("If you wish to convert Fahrenheit to Celsius, Press 1");
Console.WriteLine("If you wish to convert Fahrenheit to Kelvin, Press 2");
...

Straightforward, though a bit repetitive. From a UI point it might be useful to just show all (both) conversion results for the given input value, so the user. i.e. if the user gives Celsius, give them both Fahrenheit and Kelvin.

if (iInput == 1) {
    var aAnswer = FtoCelsius(dInput);
    Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Celsius", dInput, aAnswer);
}

Here, you could just shove the display string into a variable and fill the unit names at runtime:

string conversiondisplay = "{0} {1} equals {2} {3}"
Console.WriteLine(conversiondisplay, srcValue, srcUnit, dstValue, dstUnit);

Note that there's no "degrees Kelvin", the unit is called just Kelvin.

static double FtoKelvin(double CTempIn)
{
    double fKel = (CTempIn - 32) * (5 / 9) + 273.15;

    return fKel;
}

For a simple routine like this, you could save almost half the source code lines with no lack of readability (but leaving more of the code visible at one time):

static double FtoKelvin(double CTempIn)
{
    return (CTempIn - 32) * (5 / 9) + 273.15;
}

Though if you really want to be smart, you only need four conversion functions: Celsius to/from Kelvin, and Fahrenheit to/from Kelvin. (or with Celsius on the middle.) Doesn't matter much here, but if you wanted to convert between angstroms, mils, inches, feet, yards, meters, chains, furlongs, miles, light-years and parsecs, you'd appreciate a common "standard" unit to convert from and to.

As for the function names, I wouldn't find it sinful to use even something like FtoC here, in the case of local helper functions that will not be called from the outside and since the units are so few that there's no place for confusion. (Of course with more units, like the 11 I listed, the latter point would not apply.)

I disagree with the naming of the functions. Can you give a good reason why condensing the names to FtoC, etc., would be helpful? It doesn't promote readability, would be awful if this were factored out to a utility class with a public interface, etc. — Matthew FitzGerald-Chamberlain, Mar 16, 2017 at 22:40
@MatthewFitzGerald-Chamberlain, the explanation was unclear, edited. Public interfaces are exactly what I meant to exclude. — ilkkachu, Mar 16, 2017 at 22:52
@MatthewFitzGerald-Chamberlain FtoC is perfectly reasonable. It isn't unnecessarily verbose. Meaning of "F" and "C" in context of temperature conversion app is obvious. — el.pescado - нет войне, Mar 17, 2017 at 7:39
As a programmer using a public interface, I would be inclined to roll my own conversion rather than trying to remember exactly how to spell FahrenheitToCelsius. — Ron Jensen, Mar 21, 2017 at 16:18

Stack Exchange Network

Temperature conversion application

8 Answers 8

Fix the type errors in the calculations

Remove the options

Variable prefixes

Boxed types and type inferrence

Style guidelines

Unused imports

Your Answer

Not the answer you're looking for? Browse other questions tagged
c#
beginner
console
unit-conversion
or ask your own question.

Linked

Hot Network Questions

Temperature conversion application

8 Answers 8

Fix the type errors in the calculations

Remove the options

Variable prefixes

Boxed types and type inferrence

Style guidelines

Unused imports

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c#beginnerconsoleunit-conversion or ask your own question.

Linked

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
c#
beginner
console
unit-conversion
or ask your own question.