NEW: My new book Pro SwiftUI is out now – level up your SwiftUI skills today! >>

Day 19 Challenge, is there a better way for this code?

Forums > 100 Days of SwiftUI

Hi guys,I'm in the challenge too and I would love to learn how you would improve my code does the code look good for you? in my opinion is too long, but it works, is there another solution?

import SwiftUI

struct ContentView: View {
    @State private var inputUnit = ""
     private let units = ["Meters", "Kilometers", "Feet", "Yards", "Miles"]

    @State private var medInicial = 0
    @State private var medFinal = 2

   private var conversion: Double {
        let unitToConvert = Double(inputUnit) ?? 0

        if medInicial == 0 { //Meters
            switch (medFinal) {
            case 1:
                // m -> km
                return unitToConvert/1000
            case 2:
                // m -> ft
                return unitToConvert*3.281
            case 3:
                // m -> yd
                return unitToConvert*1.094
            case 4:
                // m -> miles
                return unitToConvert/1609
            default:
                // m -> m
                return unitToConvert
            }
        } else if medInicial == 1 { //Kilometers
            switch (medFinal) {
            case 0:
                // km -> m
                return unitToConvert*1000
            case 2:
                // km -> ft
                return unitToConvert*3281
            case 3:
                // km -> yd
                return unitToConvert*1094
            case 4:
                // km -> miles
                return unitToConvert/1.609
            default:
                // km -> km
                return unitToConvert
            }
        }else if medInicial == 2 { //Feet
            switch (medFinal) {
            case 0:
                // ft -> m
                return unitToConvert/3.281
            case 1:
                // ft -> km
                return unitToConvert/3281
            case 3:
                // ft -> yd
                return unitToConvert/3
            case 4:
                // ft -> miles
                return unitToConvert/5280
            default:
                // ft -> ft
                return unitToConvert
            }
        } else if medInicial == 3 { //Yards
        switch (medFinal) {
        case 0:
            // yd -> m
            return unitToConvert/1.094
        case 1:
            // yd -> km
            return unitToConvert/1094
        case 2:
            // yd -> ft
            return unitToConvert*3
        case 4:
            // yd -> mi
            return unitToConvert/1760
        default:
            // yd -> yd
            return unitToConvert
        }
    } else if medInicial == 4 { //Miles
        switch (medFinal) {
        case 0:
            // mi -> m
            return unitToConvert*1609
        case 1:
            // mi -> km
            return unitToConvert*1.609
        case 2:
            // mi -> ft
            return unitToConvert*5280
        case 4:
            // mi -> miles
            return unitToConvert*1760
        default:
            // mi -> mi
            return unitToConvert
        }
    }

        return unitToConvert
    }

    var body: some View {
        NavigationView {
            Form {
                Section(header: Text("Input")) {
                TextField("Input Number Here", text: $inputUnit)
                    Picker("Input Unit", selection: $medInicial) {
                        ForEach(0..<units.count) {
                            Text("\(self.units[$0])")
                            }
                        }
                .pickerStyle(SegmentedPickerStyle() )
                }
                 Section(header: Text("Output")) {
                    Picker("Output Unit", selection: $medFinal) {
                        ForEach(0..<units.count) {
                           Text("\(self.units[$0])")
                           }
                       }
                    .pickerStyle(SegmentedPickerStyle() )

                    Text("\(conversion, specifier: "%.4g")")

                }

            }.navigationBarTitle("Length Converter")
        }
    }
}

1      

hi,

you might want to take a look at Paul's How to convert units using Unit and Measurement article. most of the conversions you are doing are already built in to Swift, as long as you keep track of the UnitLengths of the input and output.

alternatively, you could collect all those constants into arrays (i think 5 x 5 will do it) so you have one conversion statement based on the indices medInicial and medFinal.

hope that helps,

DMG

2      

@delawaremathguy can you please teach me how to do it using the Built in swift conversion ?

1      

Hacking with Swift is sponsored by Play

SPONSORED Play is the first native iOS design tool created for designers and engineers. You can install Play for iOS and iPad today and sign up to check out the Beta of our macOS app with SwiftUI code export. We're also hiring engineers!

Click to learn more about Play!

Sponsor Hacking with Swift and reach the world's largest Swift community!

I have to agree that doing this with the built-in measurements and using it's conversion makes for much more elegant code. Especially if you have already covered Dictionaries.

Your dictionary would have Strings for keys (the same words as in your array) and the mesurement (UnitLength) for values.

You can then just use a function that would do the conversion, by using the array elements to determine the UnitLength.

If you read the article linked above, and feel you still need more details from me, let me know, and I will hop in and share some bits.

In closing, I just want to say congratulations on making it work. It's a great place to start.

1      

hi,

i had originally put this together for @sugertag, although i'm happy that @MarcusKay also jumped in on this as well, since there's always more than one way to do these things.

my suggestion, using built-in conversion units, can be simplified in the case of converting from meters to feet (the initial situation specified by medInicial and madFinal) by writing:

private var conversion: Double {
  let incomingValue: Double = Double(inputUnit) ?? 0
  let incomingUnitType: UnitLength = UnitLength.meters
  let unitToConvert: Measurement<UnitLength> = Measurement(value: incomingValue, unit: incomingUnitType)
  let outgoingUnitType: UnitLength = UnitLength.feet
  let convertedUnit: Measurement<UnitLength> = unitToConvert.converted(to: outgoingUnitType)
  return convertedUnit.value
}

the code is a little wordy, since i emphasized the types above so that you can see how Swift supports Units and Measurement, and see the conversion steps of String to Double, Double to Measurement, Measurement to Measurement, and lastly (by using .value) of Measurement to Double.

to generalize, without introducing a whole lot of if and case, keep an array of the UnitLength types as a simple array of five values.

private let lengthTypes: [UnitLength] = [.meters, .kilometers, .feet, .yards, .miles]

now use medInicial as an index into lengthTypes to pick out the intended incomingUnitType, and use medFinal to similarly pick out the intended outgoingUnitType.

hope that helps,

DMG

3      

@delawaremathguy is right. An array would be simpler than a Dictionary. His comment is great at showing what is happening step by step before the simplification. So I would recommend going through it first.

And just in case you want the simplified version:

private var conversion: Double {
    let unitToConvert = Measurement(value: Double(inputUnit) ?? 0, unit: lengthTypes[medInicial])
    return unitToConvert.converted(to: lengthTypes[medFinal]).value
}

The key thing to remember is that your lengthTypes array should be setup in the same way as your units array.

3      

Thank you for all the above comments. @delawaremathguy you really simplified it so it was much easier to understand the process. @MarcusKay thank you for the simplified version also. With both i eventually understood the whole process.

1      

@alvarovs89, I did mine a little differently writing functions to convert to a base unit (Kelvin in my solution) and then using a dictionary to lookup the converters from the source units to Kelvin and from Kelvin to the destination units. I guess it's a similar idea to Units and Measurements but not as general purpose.

import SwiftUI

struct ContentView: View {
    @State private var srcUnit = 0
    @State private var dstUnit = 1
    @State private var srcValue = "100"
    private let units = ["Deg", "Cels", "Kelv"]
    private var toKelvin = ["Deg": degreesToKelvin, "Cels": celsiusToKelvin, "Kelv": identity]
    private var fromKelvin = ["Deg": kelvinToDegrees, "Cels": kelvinToCelsius, "Kelv": identity]

    var body: some View {
        Form {
            Text("Temperature Converter")
                .font(.headline)
            Section(header: Text("Convert from")) {
                TextField("Temperature", text: $srcValue)
                Picker("Source units", selection: $srcUnit) {
                    ForEach(0 ..< units.count) {
                        Text(units[$0])
                    }
                }
                .pickerStyle(SegmentedPickerStyle())
            }
            Section(header: Text("Convert to")) {
                Picker("Destination units", selection: $dstUnit) {
                    ForEach(0 ..< units.count) {
                        Text(units[$0])
                    }
                }
                .pickerStyle(SegmentedPickerStyle())

                Text("\(srcValue) \(units[srcUnit]) is equivalent to \(convertedValue, specifier: "%.2f") \(units[dstUnit])")
            }
        }
    }

    var convertedValue: Double {
        let srcTemp = Double(srcValue) ?? 0
        let kelvTemp = toKelvin[units[srcUnit]]!(srcTemp)
        let dstTemp = fromKelvin[units[dstUnit]]!(kelvTemp)
        return dstTemp
    }

}

func celsiusToKelvin(celsius: Double) -> Double {
    return celsius + 273
}
func degreesToKelvin(degrees: Double) -> Double {
    return (degrees - 32) * 5 / 9 + 273
}
func kelvinToDegrees(kelvin: Double) -> Double {
    return (kelvin - 273) * 9 / 5 + 32
}
func kelvinToCelsius(kelvin: Double) -> Double {
    return kelvin - 273
}
func identity(temp: Double) -> Double {
    return temp
}

struct ContentView_Previews: PreviewProvider {
    static var previews: some View {
        ContentView()
    }
}

1      

@david149 The benefits of using Units and Measurement are that on the one hand, you don't need to have multiple functions and therefore it simplifies the whole process. On the other hand, it minimizes the risk of errors. If your formula contains a single digit wrong Xcode won't flag it and you end up with faulty conversions.

That being said, you can still simplify your code and get some "general purpose" by removing the 2 dictionaries and changing your functions to the following:

func getKelvin(from temp: Double) -> Double {
    //if the source unit is Kelvin, then no conversion needed
    guard srcUnit != 2 else { return temp }

    if srcUnit == 0 {
        return (temp - 32) * 5 / 9 + 273
    } else {
        return temp + 273
    }
}

func convertFrom(kelvin temp: Double) -> Double {
    //if the destination unit is Kelvin, then no conversion needed
    guard dstUnit != 2 else { return temp }

    if dstUnit == 0 {
        return (temp - 273) * 9 / 5 + 32
    } else {
        return temp - 273
    }
}

And then all you have to do is change your convertedValue to this:

var convertedValue: Double {
    let srcTemp = Double(srcValue) ?? 0
    return convertFrom(kelvin: getKelvin(from: srcTemp))
}

It is not necessarily the most elegant solution, but it does the job as you intended it. Also, in order to provide a bit more clarity to your functions, then our conditions could be set to the array instead:

guard units[dstUnit] != "Kelv" else { return temp }

This way, when we read the guard statement we know precisely what the unit is, instead of having to scroll up and remind ourselves of it. You can do the same thing for the if... statements.

The beauty of programming is the many ways you can solve the same puzzle. Which is a great way to practice actually. Once you get your code working properly, try to see how you can improve it. That, in my experience, has been the best way for me to learn.

1      

Thanks, @MarcusKay. I like that approach. I didn't really like my use of dictionaries but was trying to avoid a nested if or switch per the OP's original question. But with the use of guard there are really only two cases to handle and a single if statement makes sense here.

1      

Since we are all learning here, I went back to the challenge and thought I'd share an alternative we haven't approached yet. Creating an array where each element is a Tuple.

private let units: [(String, UnitLength)] = [("Meters", .meters), ("Kilometers", .kilometers), ("Feet", .feet), ("Yards", .yards), ("Miles", .miles)]

This has a couple of benefits as I hope is apparent below, but first, some information. In order to access the values of the tuple seperately, we use the following: units[0].0 this will access the String or the first part. Notice we used .0 to access the first part and therefore, we can use .1 to access the second part which is a UnitLength.

So when we create the picker, we are still using the ForEach with the units array and just access the string by simply adding .0 to the end. Anyway here's an alternative using the Tuples array and Units + Measurement to avoid having to hardcode the conversions.

struct ContentView: View {

    private let units: [(String, UnitLength)] = [("Meters", .meters), ("Kilometers", .kilometers), ("Feet", .feet), ("Yards", .yards), ("Miles", .miles)]

    @State private var inputAmount = ""
    @State private var startUnit = 0
    @State private var endUnit = 1

    private var convertedValue: Double {
        let unitToConvert = Measurement(value: Double(inputAmount) ?? 0, unit: units[startUnit].1)
        return unitToConvert.converted(to: units[endUnit].1).value
    }

    var body: some View {
        NavigationView {
            Form {
                Section(header: Text("Input")) {
                    TextField("Enter Amount:", text: $inputAmount)
                           .keyboardType(.decimalPad)
                    Picker("Starting Unit", selection: $startUnit) {
                        ForEach(0..<units.count) {
                            Text("\(self.units[$0].0)")
                        }
                    }
                    .pickerStyle(SegmentedPickerStyle())
                }

                Section(header: Text("Output")) {
                    Picker("End Unit", selection: $endUnit) {
                        ForEach(0..<units.count) {
                            Text("\(self.units[$0].0)")
                        }
                    }
                    .pickerStyle(SegmentedPickerStyle())

                    Text("\(convertedValue, specifier: "%.4g")")
                }
            }
        }
    }
}

What I like about this approach is that, by coupling the name (String) and the unit type (UnitLength) in a tuple, it allows us to add as many units as we want later on, without having to edit 2 different properties or arrays. Just add a new tuple, of the same type. In the above example (String, UnitLength). Also, the order no longer matters. You can shuffle the array any way you want, and the result remains unaffected. Good times 😉.

edit: just to be clear, "the result remains unaffected", refers to the fact that we don't need to change anything in our code if we shuffle the array around. The displayed units will change their order yes, but that has no impact on conversion.

Hopefully this provides some insight and can be helpful to anyone reading this, maybe for some project you might be working on.

2      

Sorry to revive this thread, but as I wanted to experiment with making it a bit more advanced, I stumbled onto something that makes the whole thing even "cleaner", Unit has a .symbol that returns a string with the symbol of the specific unit, which means we no longer need to use the tuple and can simply have an array of our chosen unit. For the length example:

struct ContentView: View {

    private let units: [UnitLength] = [.meters, .kilometers, .feet, .yards, .miles]

    @State private var inputAmount = ""
    @State private var startUnit = 0
    @State private var endUnit = 1

    private var convertedValue: Double {
        let unitToConvert = Measurement(value: Double(inputAmount) ?? 0, unit: units[startUnit])
        return unitToConvert.converted(to: units[endUnit]).value
    }

    var body: some View {
        NavigationView {
            Form {
                Section(header: Text("Input")) {
                    TextField("Enter Amount:", text: $inputAmount)
                           .keyboardType(.decimalPad)
                    Picker("Starting Unit", selection: $startUnit) {
                        ForEach(0..<units.count) {
                            Text(self.units[$0].symbol)
                        }
                    }
                    .pickerStyle(SegmentedPickerStyle())
                }

                Section(header: Text("Output")) {
                    Picker("End Unit", selection: $endUnit) {
                        ForEach(0..<units.count) {
                            Text(self.units[$0].symbol)
                        }
                    }
                    .pickerStyle(SegmentedPickerStyle())

                    Text("\(convertedValue, specifier: "%.4g")")
                }
            }
        }
    }
}

Notice how we remove the tuple access for converting, and in the Text view for the picker, we just add .symbol and no string interpolation is needed, because symbol returns a String.

I feel this is much better. Hope that helps!!

3      

Hacking with Swift is sponsored by Play

SPONSORED Play is the first native iOS design tool created for designers and engineers. You can install Play for iOS and iPad today and sign up to check out the Beta of our macOS app with SwiftUI code export. We're also hiring engineers!

Click to learn more about Play!

Sponsor Hacking with Swift and reach the world's largest Swift community!

Archived topic

This topic has been closed due to inactivity, so you can't reply. Please create a new topic if you need to.

All interactions here are governed by our code of conduct.

 
Unknown user

You are not logged in

Log in or create account
 

Link copied to your pasteboard.