# SI unit converter

I recently started "coding" and this is one of my first "projects". It is supposed to be an SI converter where you can type a value, its unit and the unit to which you want it to be converted.

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

namespace Program
{
class Program
{
static void Main()
{

decimal one = 1;
decimal two = 0.001m;
decimal three = 0.000001m;
decimal four = 0.000000001m;
decimal five = 0.000000000001m;
int y = 0;

while (y < 1)
{

Console.WriteLine("\nFactors: \n1.One \n2.Milli(m)\n3.Micro(µ)\n4.Nano(n)\n5.Pico(p)\nEnter factor: ");

if (factor == 1)
{
factor = one;
}
else if (factor == 2)
{
factor = two;
}
else if (factor == 3)
{
factor = three;
}
else if (factor == 4)
{
factor = four;
}
else if (factor == 5)
{
factor = five;
}

Console.WriteLine("\nFactors: \n1.One \n2.Milli(m)\n3.Micro(µ)\n4.Nano(n)\n5.Pico(p)\nEnter the second factor: ");

if (factor2 == 1)
{
factor2 = one;
}
else if (factor2 == 2)
{
factor2 = two;
}
else if (factor2 == 3)
{
factor2 = three;
}
else if (factor2 == 4)
{
factor2 = four;
}
else if (factor2 == 5)
{
factor2 = five;
}

answer = value * factor / factor2;

Console.WriteLine("Go again?\nY / N");
if (ans == 'n')
{
y = 8;
}
if (ans == 'y')
{

y = 0;
Console.Clear();

}
}
}
}
}
}


Can you give me some opinions about the code and how I can improve?

• Welcome to Code Review! I have rolled back the last edit. Please see what you may and may not do after receiving answers. Dec 2, 2015 at 20:51
• As an aside, since you have some interest in unit conversion and .NET, you may also be interested to look at F# units of measure. Dec 3, 2015 at 19:07

I'd just start making your factor namings consistent with the SI factors, like

        decimal unit = 1;
decimal milli = 0.001m;
decimal micro = 0.000001m;
decimal nano = 0.000000001m;
decimal pico = 0.000000000001m;


And there are even more like

        decimal kilo = 1000.0;
decimal mega = 1000000.0;
decimal deci = 0.1m;
decimal centi = 0.01m;


This would make your code's intent much more readable and concise when having statements as follows:

if (factor == 1) {
factor = unit;
}


Also I'd separate users choice and factors chosen go to different variables like:

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

switch(choice) {
case 1:
factor = unit;
break;
case 2:
factor = milli;
break;
// etc.
}


To make a clear separation between user input and the actual factors chosen. That will make the code even more readable, maintainable and concise.

Also, since you've been asking for c# specifically, you can have the user input as a string directly, without need to map from a number:

Console.WriteLine("\nFactors: \nUnit \nMilli(m)\nMicro(µ)\nNano(n)\nPico(p)\nEnter factor: ");

switch(choice) {
case "Unit":
factor = unit;
break;
case "Milli":
factor = milli;
break;
// etc.
}


or alternatively lookup the factor values from a properly initialized Dictonary<string, double>.

• Yes I'm thinking about doing the rest (Kilo, Mega, Giga ,Tera ) but I don't know if I can do it :D Dec 2, 2015 at 19:33
• I tried but I can't combine decimal with switch... Or at least I don't know how. Dec 2, 2015 at 19:34
• The switch is just what you're asking the user and what unit the want to have converted into another. So add just more numbers, and interpretet these mapping to factors internally. In general keep that input not directly related to values given from input. Dec 2, 2015 at 19:40
• @BatmanB. I was just answering from a straight forward POV how to improve your existing code. Of course for something production ready, you should go with Caridorc's generic solution, which avoids if/else or swich/case at all. Dec 2, 2015 at 20:07
• if (factor == 1) { factor = unit; } seems kinda useless. I don't know if it's that good to promote it. Aside from that, great first answer! Welcome!
– Nic
Dec 3, 2015 at 0:36

I suggest a simplification from:

        decimal one = 1;
decimal two = 0.001m;
decimal three = 0.000001m;
decimal four = 0.000000001m;
decimal five = 0.000000000001m;
int y = 0;

while (y < 1)
{

Console.WriteLine("\nFactors: \n1.One \n2.Milli(m)\n3.Micro(µ)\n4.Nano(n)\n5.Pico(p)\nEnter factor: ");

if (factor == 1)
{
factor = one;
}
else if (factor == 2)
{
factor = two;
}
else if (factor == 3)
{
factor = three;
}
else if (factor == 4)
{
factor = four;
}
else if (factor == 5)
{
factor = five;
}


to

int y = 0

while (y < 1)
{

Console.WriteLine("\nFactors: \n1.One \n2.Milli(m)\n3.Micro(µ)\n4.Nano(n)\n5.Pico(p)\nEnter factor: ");

factor = Math.pow( 10, ( - ( (exponent - 1) * 3 ) ) )


This makes use of the fact that the coefficient are evenly spaced on a log scale, where each coefficient is the previous to the -10**3

Some context

A number $x$ in scientific notation is in the form:

$$x = a * Pow(10, b)$$

• Where $a$ is a number from $0$ to $9.9$
• Where $b$ is an integer

Now the various prefixes that you listed in your program are each one $1000$ times smaller than the other.

This means that to get from one prefix to the next you have to multiply it by $Pow(10, -3)$

So given n you multiply it by $Pow(10, -3 * n)$ where $n$ is the input number and you get the $n-th$ suffix.

The $- 1$ just goes from 1-indexing to 0-indexing.

And a simplification in the form of removing the y variable completely and modifying the end to:

        if (ans == 'n')
{
return;
}
if (ans == 'y')
{
Console.Clear();
}


Returning immediately is also easier to understand that setting a counter declared tens of lines ago to 8 (just out of curiosity: why 8 and not 1 or 2?)

• Wow that is way more simple than mine! Though I can't really understand it :D Dec 2, 2015 at 19:24
• @BatmanB. it is easier than it looks, do you know scientific notation? Dec 2, 2015 at 19:25
• Kinda ... we are doing "transformations"? with SI units in school so I decided I can try to make some kind of converter which can do it for me :D Dec 2, 2015 at 19:29
• @BatmanB. I added some context tell me if it is any clearer Dec 2, 2015 at 19:36
• @Caridorc Thanks for the explanation I pretty much got it...(about the 8 I don't really know why I just did it Dec 2, 2015 at 19:45

This is going to be a longer example, and I'm just dealing with the conversion logic here because I think it's the most instructive. It's also not a "real world" example in the sense that the specific case you provided doesn't justify anything close to this much structure, but you could very easily find yourself heading in this direction in a production-scale app.

The code I've provided can be used to handle the input and the desired factor.

See the conversion and multiple codes here

# Key Points

• Again, this is a "teaching example". As a beginner you should not expect your code to look anything like this. One step at a time :)
• Related data belongs together in the same type. The multiple doesn't make sense without it's prefix name.

## Classes!

It's generally a good idea to create types that represents sets of values that belong together. The SI multiples are example of this. The name "nano" is associated with its factor. Because they all form a composite entity or idea, they make the most sense in a type (struct or class), which is the Multiple type in the code I provided.

public struct Multiple
{
public Multiple(string name, int exponent)
{
Name = name;
Exponent = exponent;
}
public string Name { get; }  // Read-Only Property
public int Exponent { get; } // Read-Only Property
public double Multipler => Math.Pow(10, Exponent);
public double ConvertToBaseValue(double inputValue)
{
return inputValue/Multipler;
}
public double ConvertFromBaseValue(double inputValue)
{
return inputValue*Multipler;
}
public UnitValue CreateValue(double inputValue)
{
var newBaseValue = ConvertToBaseValue(inputValue);
return new UnitValue(this, newBaseValue);
}
public static Multiple Singular => new Multiple(null, 0);
public static Multiple Milli => new Multiple("Milli", -3);
public static Multiple Micro => new Multiple("Micro", -6);
public static Multiple Nano => new Multiple("Nano", -9);
public static Multiple Pico => new Multiple("Pico", -12);
}


Similarly, your input with two questions (what is the value? what unit do I want to see it in?) then forms the basis of the UnitValue type. If you separate the two from each other they don't make a lot of sense.

public class UnitValue
{
public UnitValue(Multiple multiple, double baseValue)
{
Multiple = multiple;
BaseValue = baseValue;
}
public Multiple Multiple { get; }
public double PrefixedValue
{
get { return Multiple.ConvertFromBaseValue(BaseValue); }
set { BaseValue = Multiple.ConvertToBaseValue(value); }
}
public double BaseValue { get; set; }
public UnitValue ConvertTo(Multiple multiple)
{
return new UnitValue(multiple, BaseValue);
}
}


## Static/Class Methods for Default Values

I've added factory static methods to the Multiple to standardize the values. You'd want all of the standard definitions built into the type, or closely nearby.

    // Members of Multiple type.
public static Multiple Singular => new Multiple(null, 0);
public static Multiple Milli => new Multiple("Milli", -3);
public static Multiple Micro => new Multiple("Micro", -6);
public static Multiple Nano => new Multiple("Nano", -9);
public static Multiple Pico => new Multiple("Pico", -12);
// Allows you to get multiples like so.
var m = Multiple.Milli;
var u = Multiple.Micro;


## Don't Repeat Yourself!

Everyone will say this over and over again. Here I think it's demonstrated by avoiding typing in tedious values with all those zeroes. It also avoids bugs where I had 0.0001 for the milli prefix.

    // Risky
public static Multiple Pico => new Multiple("Pico", 0.000000000001);
// Safer
public static Multiple Pico => new Multiple("Pico", -12);


## Property Accessors

Instead of making the user use a special method to update the base value or prefixed value, I've made the decision to make the BaseValue property an auto-property ( which means I don't need to write boilerplate code for the gettor and settors), and had the PrefixedValue wired up to it. This way you can make the lives of the users of the type somewhat easier, at the price of making them responsible to know when to use the different values and why.

    public double PrefixedValue
{
get { return Multiple.ConvertFromBaseValue(BaseValue); }
set { BaseValue = Multiple.ConvertToBaseValue(value); }
}


## Get Only Properties

Because the Multiple data types belong together, it doesn't make any sense for the client to update them. Therefore to prevent data corruption I've made them get only properties that are set at the time the object is instantiated.

    // Constructor assigns the values when instantiated.
public Multiple(string name, int exponent)
{
Name = name;
Exponent = exponent;
}
public string Name { get; }  // Can only be set when instantiated.
public int Exponent { get; } // Can only be set when instantiated.


# Summary

These are some of the things you'd want to consider as you move forward. I've skipped other things that are important to understand, but they can wait until you have a lot more experience:

• Program to the interface, not to the implementation.
• Using inheritance to specialize (if I wanted to handle imperial weights).
• Using reflection to iterate over all the predefined multiples so you could change the defined multiples and your UI would update automatically.
• Unit Testing your code to validate design.

I hope that wasn't too scary! If anyone has any feedback I'd love to hear it.

# Edit 1 : Adding Usage Example

I've amended my gist here to include an example of the Main program, if it were using the classes I've written. I didn't go through a full refactoring exercise there except to remove duplication, and to push the specifics of logging into the Log method.

## Why Replace Console.WriteLine()? It's a Platform Function

Indeed it is. But you're still repeating it all over the place. Imagine if you wanted to change the way you log things? Or add a standard format?

If you keep the implementation away from the users of the logging function, then you can change it whenever you want. If you wanted to replace the console logging function with a log file, you could change it in one place. Without this abstraction you'd be updating literally dozens of references.

    // "Brittle" Code
Console.WriteLine("Bob");  // In one file
Console.WriteLine("Says"); // In another file
Console.WriteLine("Hi!");  // In a third file

// Abstracted Code
Log("Bob");
Log("Bob");

private static void Log(string message)
{
Console.WriteLine(message);
}


One Final Thing You also want to consider what happens if the user types something weird in. This example doesn't include any error handling around user entry.

• A bit too unspecific without leaving examples for a beginner, that all might just go beyond their already gained concepts so far. Though I agree with all of the points you've given. Dec 2, 2015 at 20:36
• Thanks @πάνταῥεῖ ! I've amended the gist accordingly, and have added the Main.cs file with my changes. Dec 3, 2015 at 0:21
• @RubberDuck You're definitely right :) That's now been done. Dec 3, 2015 at 1:59
• In my opinion this is best answer here. To fix just few minor issues when there is a bigger design flaw is useless. Especially because OP is beginner then he should know how things should be done otherwise...what's the point to ask on CR?! Dec 3, 2015 at 8:57
• Thanks but I can't really get the classes... Dec 3, 2015 at 12:08