0

I'm trying to break out of this listener once this condition if myDay == self.daysOfWeek[self.picker.selectedRow(inComponent: 0)] is met, but I'm not getting the results that I'm looking for.

Once the condition is met, it just continues to loop through all the documents. How do I code this up properly?

let addAction = UIAlertAction(title: "Add Workout", style: .default) { (UIAlertAction) in

    if self.dayCount != 0 {
        print("\(self.dayCount)")

        self.colRef.addSnapshotListener { (querySnapshot, err) in

            if let err = err
            {
                print("Error getting documents: \(err)");
            }
            else
            {
                for document in querySnapshot!.documents {
                    let myData = document.data()
                    let myDay = myData["dow"] as? String ?? ""

                    print(self.daysOfWeek[self.picker.selectedRow(inComponent: 0)])
                    print(myDay)
                    if myDay == self.daysOfWeek[self.picker.selectedRow(inComponent: 0)] {
                        containsDay = true
                        print(containsDay)
                        dayId = document.documentID
                        workoutColRef = Firestore.firestore().collection("/users/\(self.userIdRef)/Days/\(dayId)/Workouts/")

                        break //This Break needs to be somewhere else???
                    }
                }
            }
        }

        if containsDay == true {
            //Create new workout and store within the selectedDay.

            workoutColRef.addDocument(data: ["workout" : "\(self.textField2.text!)", "dayRef" : "\(dayId)"])

            self.loadDays()

        } else {
            //Create new day as well as a new workout, and store the workout within the day.

            let newDayRef = self.colRef.addDocument(data: ["dow" : "\(self.daysOfWeek[self.picker.selectedRow(inComponent: 0)])"])

            Firestore.firestore().collection("/users/\(self.userIdRef)/Days/\(newDayRef.documentID)/Workouts/").addDocument(data: ["workout" : "\(self.textField2.text!)", "dayRef" : newDayRef])

            newDayRef.getDocument { (docSnapshot, err) in
                if let err = err
                {
                    print("Error getting documents: \(err)");
                }
                else
                {
                    let myData = docSnapshot!.data()
                    let myDay = myData!["dow"] as? String ?? ""
                    self.daysArray.append(myDay)
                }
            }

            self.dayIdArray.append(newDayRef.documentID)

            self.loadDays()

        }
    } else {
        self.dayCount += 1 //If there are no days/workouts, we create new day as well as a new workout, and store the workout within the day.

        let newDayRef = self.colRef.addDocument(data: ["dow" : "\(self.daysOfWeek[self.picker.selectedRow(inComponent: 0)])"])

        Firestore.firestore().collection("/users/\(self.userIdRef)/Days/\(newDayRef.documentID)/Workouts/").addDocument(data: ["workout" : "\(self.textField2.text!)", "dayRef" : newDayRef])


        newDayRef.getDocument { (docSnapshot, err) in
            if let err = err
            {
                print("Error getting documents: \(err)");
            }
            else
            {
                let myData = docSnapshot!.data()
                let myDay = myData!["dow"] as? String ?? ""
                self.daysArray.append(myDay)
            }
        }

        self.dayIdArray.append(newDayRef.documentID)

        self.loadDays()   
    }
}
mkrieger1
  • 19,194
  • 5
  • 54
  • 65
  • I wrote an answer below, but then looked up some Swift info and your `break` should break from the `for in` as far as I can see. How did you determine that it continues processing? – Frank van Puffelen Apr 08 '20 at 01:52
  • @FrankvanPuffelen I updated my code to show the entire action. I determine that it continues processing because it prints "true", twice when I only call the action once. –  Apr 08 '20 at 02:59
  • 1
    You might want to put a breakpoint on that statement and step through the code in a debugger. As far as I can see (but I'm definitely not a Swift expert), the `break` should abort the loop. But stepping through it will most quickly show you if that's the case. If not, my answer migth still provide a workaround, even though I think it *should* not be needed. – Frank van Puffelen Apr 08 '20 at 03:08
  • 1
    There are a couple of issues here but most importantly is that this line `if containsDay == true` will execute immediately following `self.colRef.addSnapshotListener` which doesn't appear to be what you want. Firebase calls are asynchronous and the code following the closure will execute before the code *in* the closure. – Jay Apr 08 '20 at 15:39

2 Answers2

1

A simple way is to keep a flag variable indicating if you've found the document:

let foundIt = false
for document in querySnapshot!.documents {
    if !foundIt {
        let myData = document.data()
        let myDay = myData["dow"] as? String ?? ""

        if myDay == self.daysOfWeek[self.picker.selectedRow(inComponent: 0)] {
            containsDay = true
            dayId = document.documentID
            workoutColRef = Firestore.firestore().collection("/users/\(self.userIdRef)/Days/\(dayId)/Workouts/")

            foundIt = true
        }
    }
}
Frank van Puffelen
  • 565,676
  • 79
  • 828
  • 807
  • I believe there's an asynchronous issue in the OP's code. Also, that for loop will continue to run even after foundIt = true, so add a break following that line. – Jay Apr 08 '20 at 15:52
1

The first issue with the code in the question is that Firestore is asynchronous.

Any code following a closure will execute before the code in the closure. It takes time for data to be retrieved from the server and the code within the closure runs after that data is retrieved.

So this line

if containsDay == true {

needs to be moved.

Frank's answer is excellent but another solution is to use escaping along with a break

Suppose we want to retrieve the uid of a user - in this case we iterate over the users node to look for Bob.

users
   uid_0
      name: "Frank"
   uid_1
      name: "Bill"
   uid_2
      name: "Bob"

Here's the code we call our function with

self.lookForBobsUid(completion: { bobsUid in
    print("Bob's uid is: \(bobsUid)")
})

and then the function that reads in all of the users, iterates over them and then when we find Bob, return Bobs uid and break out of the loop.

func lookForBobsUid(completion: @escaping ( (String) -> Void ) ) {
    let usersCollection = self.db.collection("users")
    usersCollection.getDocuments(completion: { snapshot, error in
        if let err = error {
            print(err.localizedDescription)
            return
        }

        guard let documents = snapshot?.documents else { return }

        for doc in documents {
            let uid = doc.documentID
            let name = doc.get("name") as? String ?? "No Name"
            print("checking name: \(name)")
            if name == "Bob" {
                print("  found Bob, leaving loop")
                completion(uid)
                break
            }
        }
    })

    print("note this line will print before any users are iterated over")
}

Note that I added a line at the end of the code to demonstrate the nature of asynchronous calls.

All of that being said, generally speaking, iterating over collections to look for something can usually be avoided.

It appears you're looking for whatever this resolves to

self.daysOfWeek[self.picker.selectedRow(inComponent: 0)]

If so, it would be advisable to query self.colRef for that item instead of iterating to find it; that will be way faster and use less resources and will also be scalable - what if there were 100,000 nodes to iterate over!

Jay
  • 34,438
  • 18
  • 52
  • 81