Simplify this foreach loop (to find min/max in a nested array) in swift

Question

I would love to get rid of the foreach loop. Currently I am doing a foreach loop to populate a temp variable to separate my array to get the min/max for each Lat/Lon.

eg: slopeLatLonArray = [ [111,111],[111.1,111.2] ]

 func drawFullRouteOverlay() {
    /// Reset Array to Nil
    vLocations = []

    /// populate vLocations as a CLLocation2D
    for index in slopeLatLonArray.indices {
      vLocations.append(CLLocationCoordinate2D(latitude: Double(slopeLatLonArray[index][0]), longitude: Double(slopeLatLonArray[index][1])))
    }
    
    /// Draw the resulting polyline
    let polyline = MKPolyline(coordinates: vLocations, count: vLocations.count)
    vcTrainMapView.addOverlay(polyline)
    
    /// Bunch of stuffs to do to get the Max/Min of Lat/Lon
    var tempLat: [Double] = []
    var tempLon: [Double] = []
    
    slopeLatLonArray.forEach {
      tempLat.append($0[0])
      tempLon.append($0[1])
    }
    
    /// Zoom to the entire route polyline
    let center = CLLocationCoordinate2D(latitude : (tempLat.min()! + tempLat.max()!) / 2,
                                        longitude: (tempLon.min()! + tempLon.max()!) / 2)
    let span = MKCoordinateSpan(latitudeDelta : (tempLat.max()! - tempLat.min()!) * 1.3,
                                longitudeDelta: (tempLon.max()! - tempLon.min()!) * 1.3)
    let region = MKCoordinateRegion(center: center, span: span)
    vcTrainMapView.setRegion(region, animated: true)
  }

Leo Dabus · Accepted Answer · 2021-01-07 04:33:27Z

You are unnecessarily iterating all your locations multiple times. First when populating vLocations. Second when populating slopeLatLonArray. Third, fourth, fifth and sixth when getting tempLat and tempLon minimum and maximum values. And another 4 times when getting them again for the span (this might be optimized by the compiler but I am not sure).

What I suggest is to get all those values during the first iteration when populating vLocations. This way you will iterate all locations only once:

func drawFullRouteOverlay() {
    guard let first = slopeLatLonArray.first, first.count == 2 else { return }
    var minLatitude = first[0]
    var maxLatitude = first[0]
    var minLongitude = first[1]
    var maxLongitude = first[1]
    vLocations = slopeLatLonArray.map {
        let latitude = $0[0]
        let longitude = $0[1]
        minLatitude = min(minLatitude, latitude)
        maxLatitude = max(maxLatitude, latitude)
        minLongitude = min(minLongitude, longitude)
        maxLongitude = max(maxLongitude, longitude)
        return .init(latitude: latitude, longitude: longitude)
    }
    /// Draw the resulting polyline
    let polyline = MKPolyline(coordinates: vLocations, count: vLocations.count)
    vcTrainMapView.addOverlay(polyline)
    
    /// Zoom to the entire route polyline
    let center = CLLocationCoordinate2D(latitude: (minLatitude + maxLatitude) / 2, longitude: (minLongitude + maxLongitude) / 2)
    let span = MKCoordinateSpan(latitudeDelta: (maxLatitude - minLatitude) * 1.3, longitudeDelta: (maxLongitude - minLongitude) * 1.3)
    let region = MKCoordinateRegion(center: center, span: span)
    vcTrainMapView.setRegion(region, animated: true)
}

Thanks! While the answer from @Josh really did help to compact the code, this solution targets the code performance (which I like and makes a lot of sense).

Josh · Accepted Answer · 2021-01-07 23:34:39Z

1

How about either .map...

var tempLat = slopeLatLonArray.map { $0[0] }
var tempLon = slopeLatLonArray.map { $0[1] }

// Could also zip to vLocations for a 1 liner
var vLocations = zip(tempLat, tempLon).map(CLLocationCoordinate2D.init)

or setting in the initial for loop...

var tempLat: [Double] = []
var tempLon: [Double] = []
for index in slopeLatLonArray.indices {
    tempLat[index] = Double(slopeLatLonArray[index][0])
    tempLon[index] = Double(slopeLatLonArray[index][1])
    vLocations.append(CLLocationCoordinate2D(latitude: tempLat[index], longitude: tempLon[index]))
}

edited Jan 7, 2021 at 23:34

answered Jan 7, 2021 at 3:46

Josh

1,7184 gold badges22 silver badges37 bronze badges

2 Comments

app4g Over a year ago

The .map is dabomb! Just what I needed to compact the code. Looks much cleaner.

Josh Over a year ago

If you're feeling extra fancy you could zip tempLat and tempLon (updated example)`

Collectives™ on Stack Overflow

Simplify this foreach loop (to find min/max in a nested array) in swift

2 Answers 2

1 Comment

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related