Storesearch - Crashes if you search same thing twice in a row

📚 Official Books iOS Apprentice
1

I’ve tested this on both the app i’ve built and the completed app packaged with the downlaod. If you search for anything, then hit enter on the search bar again to kick off another search of the same exact term, it will crash the app. It fails on the

case .notSearchedYet:
fatalError(“Should never get here”)

in the cellForRowAt in SearchViewController. Any ideas how to correct this?

Chris

2

This is a bug in the book that I only recently managed to track down.

It happens because the second search returns really fast (using cached results_, before the table view has had a chance to show the activity indicator. Then in Search.swift the state gets changed to .notSearchedYet and – if you’re unlucky – at that moment UIKit will also reload the table view in an attempt to show the activity indicator. But now state is .notSearchedYet and the app crashes on that "Should never get here" line.

This is a typical example of a “race condition” bug that can happen when you’re using multiple threads (or Grand Central Dispatch queues in this case).

The fix is to only change state in the main thread. The changes to Search.swift look like this (indicated by // XXX):

      dataTask = session.dataTask(with: url, completionHandler: {
        data, response, error in
        
        var newState = State.notSearchedYet   // XXX
        var success = false

        if let error = error as? NSError, error.code == -999 {
          // XXX (this bit may actually not matter since we only cancel to
          // start a new serarch anyway)
          DispatchQueue.main.sync {
            self.state = .notSearchedYet   // XXX
          }
          return   // Search was cancelled
        }
        
        if let httpResponse = response as? HTTPURLResponse,
          httpResponse.statusCode == 200,
          let jsonData = data,
          let jsonDictionary = self.parse(json: jsonData) {
          
          var searchResults = self.parse(dictionary: jsonDictionary)
          if searchResults.isEmpty {
            newState = .noResults   // XXX
          } else {
            searchResults.sort(by: <)
            newState = .results(searchResults)  // XXX
          }
          success = true
        }

        DispatchQueue.main.async {
          self.state = newState                  // XXX
          UIApplication.shared.isNetworkActivityIndicatorVisible = false
          completion(success)
        }
      })

If you make these changes, the code should work OK. :smile:

3

Should this line be var newState: State = .notSearchedYet?

Wont I now have to read the value of newState instead of state? This never gets past the loading spinner.

4

Whoops, I didn’t copy-paste the fix correctly. I have edited my answer, so it should all be there now. (You may need to scroll the code to see all of it.)

5

That fixed it! thanks for the help!

Chris

6

@hollance I am on page 214 and seems that I have to complete the section (page 222) before changing this code or the first line we changes gave the error “Reference to member 'notSearchedYet cannot be resolved without a contextual type” for the line:

var newState = .notSearchedYet // XXX

I tried to figure out what I need to add later in the section to Search.swift to get the above code to work properly, but I couldn’t find it.

Could you explain why we get this error and what needs to be added from the other area of the section to get this working at page 199?

7

I had to write it as

var newState: State = .notSearchedYet

Then it would compile and work.

8

Hmm, I must have messed up my copy-paste (again!). The line should be:

var newState = State.notSearchedYet

Without the State type, the compiler doesn’t know where to look for .notSearchedYet. I’ll fix it in the above code too.

9

@hollance @oochr1soo Thank you! The error makes sense then.

10

@hollance is there an advantage to using var newState = State.notSearchedYet vs var newState: State = .notSearchedYet

11

Either one will work just fine. It’s two different ways of writing the exact same thing. :slight_smile:

12

Hi there,

had a similar issue where it would crash after the first search already. What I found is that
the reloadData() which is called in SearchViewControllers SearchBar Extension performSearch().

Removing that line fixed the problem for me.

But … I still do not understand why the system would jump into cellForRowAt if it had already called
numberOfRowsInSection which should have returned “0” … Do I get something wrong ?

Regards,
Mark

13

I think I understand it now after reading through the thread :slight_smile: Thanks guys, Mark