CODEWITHSUNDEEP
swift
beginner992
beginner992

Reputation: 719

CompactMap and an array in an array

I want to combine two arrays thanks to compactMap. There are two lists - Stations and Sensors. In the Sensor object there is a property named: stationId. It helps to combine Sensors with Stations, but there is a problem.

Some sensors are owned by the same stationId. So one Station can own several Sensors. Three, five, or even one and so on. But I can't do it by compactMap. I always have only one Sensor per Station. How to fix this? How to make more Sensors per Station in my case?

 func setAddItemList(stations: [Station], sensors: [Sensor]) {
    
    if stations.count > 0 && sensors.count > 0 {
        
        addItemList = stations.compactMap { station in
            guard let sensor = sensors.first(where: { $0.stationId == station.id}) else {
                return nil
            }
            return AddItem(id: station.id, stationId: sensor.stationId, cityName: station.city!.name, addressStreet: station.addressStreet!, sensor: ([SensorItem(id: sensor.id, stationId: sensor.stationId, param: (ParamItem(paramName: sensor.param!.paramName, paramFormula: sensor.param!.paramFormula, paramCode: sensor.param!.paramCode, idParam: sensor.param!.idParam)))]) )
        }
        if addItemList.count > 0 {
            addItem.onNext(addItemList)
        }
    }
}

Upvotes: 0

Views: 275

Answers (2)

Alexander
Alexander

Reputation: 63137

One of these lines is 375 lines long, with 2 sets of unnecessary braces. Before I even do anything, just tidying this up makes it much easier to read:

func setAddItemList(stations: [Station], sensors: [Sensor]) {
    if sensors.count > 0 {
        addItemList = stations.compactMap { station in
            guard let sensor = sensors.first(where: { $0.stationId == station.id}) else {
                return nil
            }
            return AddItem(
                id: station.id,
                stationId: sensor.stationId,
                cityName: station.city!.name,
                addressStreet: station.addressStreet!,
                sensor: [
                    SensorItem(
                        id: sensor.id,
                        stationId: sensor.stationId,
                        param: ParamItem(
                            paramName: sensor.param!.paramName,
                            paramFormula: sensor.param!.paramFormula,
                            paramCode: sensor.param!.paramCode,
                            idParam: sensor.param!.idParam
                        )
                    ),
                ]
            )
        }
        if addItemList.count > 0 {
            addItem.onNext(addItemList)
        }
    }
}

The check for stations.count > 0 does nothing, so I removed it.

I would replace this entire approach. compactMap is for mapping and filtering out nils. In each invvocation of its closure, you're doing a linear scan through the sensors to find the first one that belongs to the station. That's slow, but as you found, it's also incorrect, because it'll always land on the same item.

Instead, I would use a dictionary to key sensors by their ID. Something like this:


func setAddItemList(stations: [Station], sensors: [Sensor]) {
    let sensorsByStationID = Dictionary(grouping: sensors, by: \.stationId)
    let sensorItemsByStationID = sensorsByStationID.mapValues { sensor in 
        return SensorItem(
            id: sensor.id,
            stationId: sensor.stationId,
            param: ParamItem( // FIXME: ParamItem is a terrible name
                paramName: sensor.param!.paramName,
                paramFormula: sensor.param!.paramFormula,
                paramCode: sensor.param!.paramCode,
                idParam: sensor.param!.idParam
            )
        )
    }

    let addItems = stations.map { station in 
        AddItem( // TODO: AddItem? What does that have to do with a "Station" or a "Sensor"?
            id: station.id,
            stationId: sensor.stationId,
            cityName: station.city!.name,
            addressStreet: station.addressStreet!,
            sensor: sensorItemsByStationID[station.id] ?? [] // FIXME: This keyword label should be `sensorItems`, not singular.
        )
    }

    if !addItems.isEmpty {
        addItem.onNext(addItems) // FIXME: this might be a misplaced responsibility in this function.
    }
}

Upvotes: 1

Joakim Danielson
Joakim Danielson

Reputation: 51821

I would use a forEach on the sensors array to match each sensor with a station

sensors.forEach { sensor in
    guard let index = stations.firstIndex(where: { $0.id == sensor.stationId }) else {
        print("Failed to find a station for sensor \(sensor.id)")
        return
    }
    stations[index].sensors.append(sensor)
}

Upvotes: 2

Related Questions