2
\$\begingroup\$

I have a form with listboxes used for unit conversions. I'm using the UnitsNet package.

Here are my questions:

  1. Is this the proper approach?
  2. If any changes must be made to this code, what are they?

enter image description here

private void listBoxControl1_SelectedIndexChanged(object sender, EventArgs e)
{
    UnitTypes enumValue = (UnitTypes)Enum.Parse(typeof(UnitTypes), listBoxControl1.SelectedItem.ToString());
    listBoxControl2.Items.Clear();
    listBoxControl3.Items.Clear();

    switch (enumValue)
    {
        case UnitTypes.Energy:
            listBoxControl2.Items.AddRange(typeof(EnergyUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(EnergyUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Flow:
            listBoxControl2.Items.AddRange(typeof(FlowUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(FlowUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Force:
            listBoxControl2.Items.AddRange(typeof(ForceUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(ForceUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Frequency:
            listBoxControl2.Items.AddRange(typeof(FrequencyUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(FrequencyUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Length:
            listBoxControl2.Items.AddRange(typeof(LengthUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(LengthUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Number:
            listBoxControl2.Items.AddRange(typeof(NumberUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(NumberUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Power:
            listBoxControl2.Items.AddRange(typeof(PowerUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(PowerUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Pressure:
            listBoxControl2.Items.AddRange(typeof(PressureUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(PressureUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Speed:
            listBoxControl2.Items.AddRange(typeof(SpeedUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(SpeedUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Temperature:
            listBoxControl2.Items.AddRange(typeof(TemperatureUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(TemperatureUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Time:
            listBoxControl2.Items.AddRange(typeof(TimeUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(TimeUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Timing:
            listBoxControl2.Items.AddRange(typeof(TimingUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(TimingUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Torque:
            listBoxControl2.Items.AddRange(typeof(TorqueUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(TorqueUnits).GetEnumValues().Cast<object>().ToArray());
            break;
        case UnitTypes.Volume:
            listBoxControl2.Items.AddRange(typeof(VolumeUnits).GetEnumValues().Cast<object>().ToArray());
            listBoxControl3.Items.AddRange(typeof(VolumeUnits).GetEnumValues().Cast<object>().ToArray());
            break;
    }
}

//This is hit when selection changes in listboxes or when value changes in 'Input Value' textbox
//This handles only Length unit as of now.
private void updateValues(object sender, EventArgs e)
{
    UnitTypes unitType = (UnitTypes)Enum.Parse(typeof(UnitTypes), listBoxControl1.SelectedItem.ToString());

    switch (unitType)
    {
        case UnitTypes.Energy:
            convertEnrgyUnit();
            break;
        case UnitTypes.Flow:
            ConvertFlowUnit();
            break;
        case UnitTypes.Force:
            ConvertForceUnit();
            break;
        case UnitTypes.Frequency:
            ConvertFrequencyUnit();
            break;
        case UnitTypes.Length:
            convertLengthUnit();
            break;
        case UnitTypes.Number:

            break;
        case UnitTypes.Power:

            break;
        case UnitTypes.Pressure:

            break;
        case UnitTypes.Speed:

            break;
        case UnitTypes.Temperature:

            break;
        case UnitTypes.Time:

            break;
        case UnitTypes.Timing:

            break;
        case UnitTypes.Torque:

            break;
        case UnitTypes.Volume:

            break;
    }
}


private void convertLengthUnit()
{
    LengthUnits energyInputVal = LengthUnits.Feet;
    LengthUnits energyOutputVal = LengthUnits.Feet;
    if (listBoxControl2.SelectedItem != null)
    {
        energyInputVal = (LengthUnits)listBoxControl2.SelectedItem;
    }
    if (listBoxControl3.SelectedItem != null)
    {
        energyOutputVal = (LengthUnits)listBoxControl3.SelectedItem;
    }

    if (listBoxControl2.SelectedItem != null && listBoxControl3.SelectedItem != null && !String.IsNullOrWhiteSpace(textEdit1.Text))
    {
        Length inputVal = Length.FromFeet(1);
        decimal outputVal = 0;
        switch (energyInputVal)
        {
            case LengthUnits.Feet:
                inputVal = Length.FromFeet(Convert.ToDouble(textEdit1.Text));
                break;
            case LengthUnits.Inches:
                inputVal = Length.FromInches(Convert.ToDouble(textEdit1.Text));
                break;
            case LengthUnits.Kilometers:
                inputVal = Length.FromKilometers(Convert.ToDouble(textEdit1.Text));
                break;
            case LengthUnits.Meters:
                inputVal = Length.FromMeters(Convert.ToDouble(textEdit1.Text));
                break;
            case LengthUnits.Miles:
                inputVal = Length.FromMiles(Convert.ToDouble(textEdit1.Text));
                break;
        }

        switch (energyOutputVal)
        {
            case LengthUnits.Feet:
                outputVal = Convert.ToDecimal(inputVal.Feet);
                break;
            case LengthUnits.Inches:
                outputVal = Convert.ToDecimal(inputVal.Inches);
                break;
            case LengthUnits.Kilometers:
                outputVal = Convert.ToDecimal(inputVal.Kilometers);
                break;
            case LengthUnits.Meters:
                outputVal = Convert.ToDecimal(inputVal.Meters);
                break;
            case LengthUnits.Miles:
                outputVal = Convert.ToDecimal(inputVal.Miles);
                break;
        }

        textEdit2.Text = outputVal.ToString();
    }
}


private void Form1_Load(object sender, EventArgs e)
{
    listBoxControl1.Items.AddRange(typeof(UnitTypes).GetEnumValues().Cast<object>().ToArray());
}
\$\endgroup\$
  • \$\begingroup\$ "Instead of manually doing some calculations in code i'm looking for some library or open source that does all these calculations for us." Would somethinhg like this work for you? \$\endgroup\$ – πάντα ῥεῖ Apr 18 '17 at 18:53
  • \$\begingroup\$ Definitely, all I want is a form that can make all conversions and i'm just a beginner, after looking at my code I thought there might definitely be a better way of writing this logic. \$\endgroup\$ – user Apr 18 '17 at 19:03
  • \$\begingroup\$ Why not use google? It can covert everything :P \$\endgroup\$ – t3chb0t Apr 18 '17 at 19:06
  • \$\begingroup\$ Well, the stuff I've been linking to is configurable via XML. All you need is to populate your ListBox controls from the defined consverters. \$\endgroup\$ – πάντα ῥεῖ Apr 18 '17 at 19:06
  • \$\begingroup\$ @t3chb0t I will ask my manager and see if he could do so :P \$\endgroup\$ – user Apr 18 '17 at 19:07
1
\$\begingroup\$

First, split your methods up into smaller units. For example, convertLengthUnit should have this general structure (and PascalCase naming):

private void ConvertLengthUnit()
{
    var lengthInputVal = LengthUnits.Feet;
    var lengthOutputVal = LengthUnits.Feet;

    if (listBoxControl2.SelectedItem != null)
    {
        lengthInputVal = (LengthUnits)listBoxControl2.SelectedItem;
    }

    if (listBoxControl3.SelectedItem != null)
    {
        lengthOutputVal = (LengthUnits)listBoxControl3.SelectedItem;
    }

    if (listBoxControl2.SelectedItem != null && listBoxControl3.SelectedItem != null && !String.IsNullOrWhiteSpace(textEdit1.Text))
    {
        textEdit2.Text = Convert(lengthInputVal, lengthOutputVal);
    }
}

Note that I extracted the actually conversion to another method. Also, please give your controls better names. Perhaps inputTypes instead of listBoxControl2. You can change these names in the Properties window.

Those switch blocks are a pain to read, and probably even more of a pain to update. Unfortunately, I don't see a better alternative at the moment, other than to extract them into, for example:

Length GetInput()
{
    switch (energyInputVal)
    {
        case LengthUnits.Feet:
            return Length.FromFeet(Convert.ToDouble(textEdit1.Text));
        case LengthUnits.Inches:
            return Length.FromInches(Convert.ToDouble(textEdit1.Text));
        // others...
    }
}

And:

GetOutput(LengthUnits outputType, Length data)
{
    switch (energyOutputVal)
    {
        case LengthUnits.Feet:
            return Convert.ToDecimal(inputVal.Feet);
        case LengthUnits.Inches:
            return Convert.ToDecimal(inputVal.Inches);
        // ...
    }
}

Now, you have a method with one responsibility, and it will probably be a little easier to make sure you update everything if you add any new types.

I would use these same principles to load the input/output types. I would create a method like so:

void AddUnits<T>()
{
    var units = typeof(T).GetEnumValues().Cast<object>().ToArray();
    listBoxControl2.Items.AddRange(units);
    listBoxControl3.Items.AddRange(units);
}

Then call it like:

switch (enumValue)
{
    case UnitTypes.Energy:
        AddUnits<EnergyUnits>();
    case UnitTypes.Flow:
        AddUnits<FlowUnits>();
        break;
    // ...
}
\$\endgroup\$
1
\$\begingroup\$

@hosch250 's answer covers a lot of good feedback, so I will focus on how you can further simplify the switch statements.

In your comments you mentioned switching to Units.NET, which allows you to do something like this:

double inputValue = 5; // Obtain from input text box
var inputUnit = LengthUnit.Meter; // Obtain from list selection
var outputUnit = LengthUnit.Centimeter; // Obtain from list selection

double outputValue = Length.From(inputValue, inputUnit).As(outputUnit); // 500

This covers the case when Length is selected.

You already have code in place to switch to a different conversion code for when Length, Mass etc is selected and you still need something like that to handle those separately, but Units.NET helps convert between units and providing an enum like LengthUnit for enumerating all possible units to convert between.

Now, I was inspired by this scenario and it's been on my mind for some time, so I went ahead and added support for dynamic unit conversion and it's now available in UnitsNet 3.70.0.

The example now becomes:

// Get quantities for populating quantity UI selector
QuantityType[] quantityTypes = Enum.GetValues(typeof(QuantityType)).Cast<QuantityType>().ToArray();

// If Length is selected, get length units for populating from/to UI selectors
LengthUnit[] lengthUnits = Length.Units;

// Perform conversion by using .ToString() on selected quantity and units
double centimeters = UnitSystem.ConvertByName(5, "Length", "Meter", "Centimeter");

I also added an alternative method ConvertByAbbreviation(), that take unit abbreviations "m" and "cm" instead.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.