How to make a function with a loop asynchronous in Swift?

Question

I am creating an application for a library. I am trying to fetch all the books the user has checked out from Firebase, but my attempts to make the function asynchronous with a DispatchGroup doesn't seem to be working. I suspect this to be because of the for-in loop found inside of the function.

    func fetchHistory() {

    if items.count > 0 {
        items.removeAll()
    }

    let myGroup = DispatchGroup()
    myGroup.enter()

    var itemNames = [String]() // this holds the names of the child values of /users/uid/items/ <-- located in Firebase Database
    guard let uid = fAuth.currentUser?.uid else {return}
    fData.child("users").child(uid).child("items").observe(.value, with: { snapshot in

        // make sure there is at least ONE item in the history
        if snapshot.childrenCount > 0 {
            let values = snapshot.value as! NSDictionary
            for i in values.allKeys {
                itemNames.append(i as! String)
            }

            print(itemNames)
            let uid = fAuth.currentUser!.uid // get the UID of the user
            for item in itemNames {
                fData.child("users").child(uid).child("items").child(item).observe(.value, with: { snapshot in
                    let values = snapshot.value as! NSDictionary
                    let bookTitle = values["title"] as! String
                    print(bookTitle)
                    let bookAuthor = values["author"] as! String
                    print(bookAuthor)
                    let bookCoverUrl = values["coverUrl"] as! String
                    print(bookCoverUrl)
                    let bookStatus = values["status"] as! String
                    print(bookStatus)
                    let bookDueDate = values["dueDate"] as! String
                    print(bookDueDate)

                    let book = Book(name: bookTitle, author: bookAuthor, coverUrl: bookCoverUrl, status: bookStatus, dueDate: bookDueDate)
                    self.items.append(book)
                })
            }
            self.booksTable.isHidden = false
        } else {
            self.booksTable.isHidden = true
        }

    })

    myGroup.leave()
    myGroup.notify(queue: DispatchQueue.main, execute: {
        self.booksTable.reloadData()
        print("Reloading table")
    })

}

Here is the output from the print() statements:

########0
Reloading table
["78DFB90A-DE5B-47DE-ADCA-2DAB9D43B9C8"]
Mockingjay (The Hunger Games, #3)
Suzanne Collins
https://images.gr-assets.com/books/1358275419s/7260188.jpg
Checked
Replace

The first two lines of output should be printed AFTER everything else has printed. I really need some help on this, I have been stuck on this for hours. Thanks!

Edit: As requested, here is my Firebase structure:

users:
  meZGWn5vhzXpk5Gsh92NhSasUPx2:
    ID: "12345"
    firstname: "Faraaz"
    items:
        78DFB90A-DE5B-47DE-ADCA-2DAB9D43B9C8
            author: "Suzanne Collins"
            coverUrl: "https://images.gr assets.com/books/1358275419s/..."
            dueDate: "Date"
            status: "Checked"
            title: "Mockingjay (The Hunger Games, #3)"
     type: "regular"

There may be better approaches to get the data you want without having to create a loop like that. With the right Firebase structure you should be able to perform a single query and return all of the books the user has checked out. Can you include a text snippet of your Firebase structure so we know what's being stored currently in Firebase? You're probably going to want to fan out/denormalize your structure which will really help. — Jay
– Jay, Commented Jan 17, 2018 at 21:19
Unrelated to your main question, note that if you decide to stay with the above pattern, I'd suggest not asynchronously appending Book values directly to the main model object driving your table view. You should populate a local array, and then only update your model object in the notify block. Right now, if the user happened to scroll after some records had appended but before the final reloadData, you could get errors about inconsistency regarding the state of the table view and the model. — Rob
– Rob, Commented Jan 17, 2018 at 23:21
@Jay I have updated the post. I spent so much time trying to figure out how to do it a better/cleaner way, but I just couldn't find a way. This was really a last resort. — itsfaraaz
– itsfaraaz, Commented Jan 17, 2018 at 23:49
@Rob Thank you so much for your help, I'll make the edits accordingly. — itsfaraaz
– itsfaraaz, Commented Jan 17, 2018 at 23:50
@Rob answer is fabulous and technically just about perfect, I added another answer as an alternative approach (for future readers). I think long-term you're going to want to adjust your structure to better suit what you are querying for. Part of making Firebase fly is denormalizing data, fanning out data and reducing observes and queries. So instead of querying/observing 1000 times to get 1000 pieces of data, replace that with a single observe or query to get those 1000 values once - it's way more efficient and will significantly increase the UI for your user. Just a thought :-) — Jay
– Jay, Commented Jan 18, 2018 at 18:28

Rob · Accepted Answer · 2018-01-17 21:08:27Z

2

A couple of issues:

The pattern is that leave must be called inside the completion handler of the asynchronous call. You want this to be the last thing performed inside the closure, so you could add it as the the last line within completion handler closure.

Or I prefer to use a defer clause, so that not only do you know it will be the last thing performed in the closure, but also:
- you ensure you leave even if you later add any "early exits" inside your closure; and
- the enter and leave calls visually appear right next to each other in the code saving you from having to visually hunt down at the bottom of the closure to make sure it was called correctly.
You also, if you want to wait for the asynchronous calls in the for loop, have to add it there, too.
A very minor point, but you might want to not create the group until you successfully unwrapped uid. Why create the DispatchGroup if you could possibly return and not do any of the asynchronous code?

Thus, perhaps:

func fetchHistory() {

    if items.count > 0 {
        items.removeAll()
    }

    var itemNames = [String]()
    guard let uid = fAuth.currentUser?.uid else {return}

    let group = DispatchGroup()

    group.enter()

    fData.child("users").child(uid).child("items").observe(.value, with: { snapshot in

        defer { group.leave() }               // in case you add any early exits, this will safely capture

        if snapshot.childrenCount > 0 {
            ...
            for item in itemNames {

                group.enter()                 // also enter before we do this secondary async call

                fData.child("users").child(uid).child("items").child(item).observe(.value, with: { snapshot in

                    defer { group.leave() }   // and, again, defer the `leave`

                    ...
                })
            }
            ...
        } else {
            ...
        }
    })

    group.notify(queue: .main) {
        self.booksTable.reloadData()
        print("Reloading table")
    }    
}

edited Jan 17, 2018 at 21:08

answered Jan 17, 2018 at 21:02

Rob

441k74 gold badges851 silver badges1.1k bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

itsfaraaz Over a year ago

Thank you for your help, but can you explain what the defer clause exactly is?

Rob Over a year ago

As The Swift Programming Language:A Swift Tour: Error Handling says "Use defer to write a block of code that is executed after all other code in the function, just before the function returns. The code is executed regardless of whether the function throws an error. You can use defer to write setup and cleanup code next to each other, even though they need to be executed at different times."

Rob Over a year ago

It's basically a nice, easy way to specify a block of code that will be executed when you exit the current scope, regardless of how you did so (e.g., whether exiting normally, an early exit within guard statement, throwing an error, etc.).

itsfaraaz Over a year ago

Thank you so much for your help.

Jay · Accepted Answer · 2018-01-18 18:33:00Z

While there is a brilliant answer from Rob, I would approach a solution from a different direction.

A book can only ever had one person check it out (at a time), but a borrower can have multiple books. Because of that relationship, simply combine who has the book with the book itself:

Here's a proposed users structure

users
  uid_0
    name: "Rob"
  uid_1
    name: "Bill"

and then the books node

books
   78DFB90A-DE5B-47DE-ADCA-2DAB9D43B9C8
     author: "Suzanne Collins"
     coverUrl: "https://images.gr assets.com/books/1358275419s/..."
     dueDate: "Date"
     status: "Checked"
     title: "Mockingjay (The Hunger Games, #3)"
     checked_out_by: "uid_0"
     check_date: "20180118"

Then to get ALL of the books that Rob has checked out and use those results to populate an array and display it in a tableview becomes super simple:

//var bookArray = [Book]() //defined as a class var

let booksRef = self.ref.child("books")
let query = booksRef.queryOrdered(byChild: "checked_out_by").queryEqual(toValue: "uid_0")
booksRef.observeSingleEvent(of: .value, with: { snapshot in
    for child in snapshot.children {
        let snap = child as! DataSnapshot
        let book = Book(initWithSnap: snap) //take the fields from the snapshot and populate the book
        self.bookArray.append(book)
    }
    self.tableView.reloadData()
})

But then you ask yourself, "self, what if I want a record of who checked out the book?"

If you need that functionality, just a slight change to the books node so we can leverage a deep query;

books
   78DFB90A-DE5B-47DE-ADCA-2DAB9D43B9C8
     author: "Suzanne Collins"
     coverUrl: "https://images.gr assets.com/books/1358275419s/..."
     dueDate: "Date"
     status: "Checked"
     title: "Mockingjay (The Hunger Games, #3)"
     check_out_history
        "uid_0" : true
        "uid_1" : true

and move the check out dates to the users node. Then you can query for any user of any book and have history of who checked out that book as well. (there would need to be logic to determine who has the book currently so this is just a starting point)

Or if you want another option, keep a separate book history node

book_history
   78DFB90A-DE5B-47DE-ADCA-2DAB9D43B9C8
      -9j9jasd9jasjd4    //key is created with childByAutoId
         uid: "uid_0"
         check_out_date: "20180118"
         check_in_date: "20180122"
         condition: "excellent"
      -Yuhuasijdijiji    //key is created with childByAutoId
         uid: "uid_1"
         check_out_date: "20180123"
         check_in_date: "20180125"
         condition: "good"

The concept is to let Firebase do the work for you instead of iterating over arrays repeatedly and having to issue dozens of calls to get the data you need. Adjusting the structure makes it much simpler to maintain and expand in the future as well - and it avoids all of the issues with asynchronous code as it's all within the closure; nice and tidy.

Collectives™ on Stack Overflow

How to make a function with a loop asynchronous in Swift?

2 Answers 2

4 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related