HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

SI unit converter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
converterunitstackoverflow

Problem

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;
using System.Threading.Tasks;

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;
decimal answer;
int y = 0;

while (y < 1)
{

Console.WriteLine("SI converter!\nPlease, enter value: ");
decimal value = Convert.ToInt32(Console.ReadLine());

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

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: ");
decimal factor2 = Convert.ToInt32(Console.ReadLine());

if (factor2 == 1)
{
factor2 = one;
answer = value * factor;
Console.WriteLine("The answer is : " + answer);
}
else if (factor2

Solution

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: ");
string choice = Console.ReadLine();

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


or alternatively lookup the factor values from a properly initialized Dictonary.

Code Snippets

decimal unit = 1;
        decimal milli = 0.001m;
        decimal micro = 0.000001m;
        decimal nano = 0.000000001m;
        decimal pico = 0.000000000001m;
decimal kilo = 1000.0;
        decimal mega = 1000000.0;
        decimal deci = 0.1m;
        decimal centi = 0.01m;
if (factor == 1) {
    factor = unit;
}
int choice = Convert.ToInt32(Console.ReadLine());

            switch(choice) {
            case 1:
                factor = unit;
                break;
            case 2:
                factor = milli;
                break;
            // etc.
            }
Console.WriteLine("\nFactors: \nUnit \nMilli(m)\nMicro(µ)\nNano(n)\nPico(p)\nEnter factor: ");
string choice = Console.ReadLine();

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

Context

StackExchange Code Review Q#112636, answer score: 16

Revisions (0)

No revisions yet.