-1

I am trying to append array(simple task I know) but for some reason my array is empty. No matter what I do. This is what I do:

I want to calculate the total balance(I should do amount * price and then sum them up).

So I thought I would make array of balances and then sum them up but the array is empty:

func getBalances(completion: @escaping (_ balancesArray: [Double])->()){
        var balancesArray = [Double]()
        for f in portfolioArray{
            getPrice(for: f.name!, in: "EUR", completion: { (price) in
                balancesArray.append(f.amount * price)
                //!!!!!!If I print here the array, it is full!!!!!!
            })
        }
        completion(balancesArray)
    }

This is the getPrice function(it returns the price, I tested):

func getPrice(for crypto: String, in fiat: String, completion: @escaping (_ price: Double)->()){
        API.getCryptoRates(cryptoSymbol: crypto, fiatSymbol: fiat) { (error, response: [CurrencyResponse]?) in
            if error == nil{
                if let prices = response{
                    guard let p = Double(prices[0].price) else { return }
                    completion(p)
                }
            }
        }
    }

So my question is, why is it empty? If I am correct then the completion should have the filled array. And there shouldn't be any thread problems.

Can somebody please lead me to the right track. Any response is appreciated!

Dávid Pásztor
  • 51,403
  • 9
  • 85
  • 116
Tarvo Mäesepp
  • 4,477
  • 3
  • 44
  • 92
  • Did you checked portfolioarray ? Is it contains elements? – Sagar Chauhan Feb 07 '18 at 15:49
  • @PaulMarshal Yes, it contains the elements. – Tarvo Mäesepp Feb 07 '18 at 15:49
  • 1
    `getPrice()` being...? Is it async? If yes, you'd need to use a dispatch_group. – Larme Feb 07 '18 at 15:50
  • Print something immediately before `completion(balancesArray)`. See whether it happens before or after "If I print here the array, it is full". – Phillip Mills Feb 07 '18 at 15:51
  • @Larme Yes it should be because Alamofire's documentation says that all the responses are on main thread already. I also tried to play with threads. – Tarvo Mäesepp Feb 07 '18 at 15:51
  • I think you need to make wait until your getPrice method with completion will completed execution. Because may be issue is there. – Sagar Chauhan Feb 07 '18 at 15:52
  • @TarvoMäesepp You didn't understood. Being in main thread does not guaranty that it's async or not. You are having misconception issue. Could you give the code of `getPrice()`? – Larme Feb 07 '18 at 15:52
  • 1
    You have a serious flaw in `getPrice`. You must call the completion handler under all conditions otherwise the caller will never know whether it worked or not. Change its completion handler to pass back an optional so the caller can determine if the call succeeded or not. – rmaddy Feb 07 '18 at 15:55

2 Answers2

2

getPrice is asynchronous. The for loop finishes long before the first call to getPrice even gets started.

You need to use DispatchGroup to ensure that the call to completion(balancesArray) isn't made until all of the calls to getPrice have completed. See What's the best way to iterate over results from an APi, and know when it's finished? for an example.

You will also need to ensure you append to balancesArray from a single thread to avoid concurrent writes. See Create thread safe array in swift for solutions.

rmaddy
  • 314,917
  • 42
  • 532
  • 579
2

The issue is that getPrice is asynchronous, so when you call completion your async functions haven't actually finished execution. You can use a DispatchGroup to solve your issue.

You should also make sure that you only write to balancesArray from a single thread to avoid concurrency issues.

Below code ensures that the completion handler of getBalances is only called once all calls to getPrice are finished, but it doesn't make the balancesArray threadsafe, you can take care of that as explained in Create thread safe array in Swift

func getBalances(completion: @escaping (_ balancesArray: [Double])->()){
    var balancesArray = [Double]()
    let group = DispatchGroup()
    for f in portfolioArray{
        group.enter()
        getPrice(for: f.name!, in: "EUR", completion: { price in
            if let p = price{
                balancesArray.append(f.amount * p)
            }
            group.leave()
        })
    }
    group.notify(queue: .main, execute: {
        completion(balancesArray)
    })
}

Your getPrice function is flawed. You need to make sure that the completion handler is called in all cases even when the result is an error. When working with network requests, you should always make completion handlers return optionals and in case of any issues, you should call completion with a nil value. If you want to know the reason for the nil value, you can also make your closure return a tuple containing an optional value and an optional Error, such as URLSession.dataTask(with:, completionHandler:) does.

func getPrice(for crypto: String, in fiat: String, completion: @escaping (_ price: Double?)->()){
    API.getCryptoRates(cryptoSymbol: crypto, fiatSymbol: fiat) { (error, response: [CurrencyResponse]?) in
        if error == nil{
            if let prices = response{
                guard let p = Double(prices[0].price) else { completion(nil); return }
                completion(p)
            } else {
                completion(nil)
            }
        } else {
            completion(nil)
        }
    }
}
Dávid Pásztor
  • 51,403
  • 9
  • 85
  • 116