40. Refactoring

Written by Matthijs Hollemans & Fahim Farook

Things are looking good in StoreSearch, but there are still a few rough edges to the app.

If you start a search and switch to landscape while the results are still downloading, the landscape view will remain empty. You can reproduce this situation by artificially slowing down your network connection using the Network Link Conditioner tool.

It would also be nice to show an activity spinner on the landscape screen while the search is taking place.

You will polish off some of these rough edges in this chapter and cover the following:

• Refactor the search: Refactor the code to put the search logic into its own class so that you have centralized access to the search state and results.
• Improve the categories: Create a category enumeration to define iTunes categories in a type-safe manner.
• Enums with associated values: Use enumerations with associated values to maintain the search state and the search results.
• Spin me right round: Add an activity indicator to the landscape view. Also add a network activity indicator to the app.
• Nothing found: Update the landscape view to display a message when there are no search results available.
• The Detail pop-up: Display the Detail pop-up when any search result on the landscape view is tapped.

Refactor the search

So how can LandscapeViewController tell what state the search is in? Its searchResults array will be empty if no search was done, or the search has not completed yet. Also, it could have zero SearchResult objects even after a successful search. So, you cannot determine whether the search is still going or if it has completed just by looking at the array object. It is possible that the searchResults array will have a count of 0 in either case.

You need a way to determine whether a search is still going on. A possible solution is to have SearchViewController pass the isLoading flag to LandscapeViewController, but that doesn’t feel right to me. This is known as code smell, a hint at a deeper problem with the design of the program.

Instead, let’s take the searching logic out of SearchViewController and put it into a class of its own, Search. Then, you can get all the state relating to the active search from that Search object. Time for some refactoring!

The Search class

➤ If you want, create a new branch for this in Git.

This is a pretty comprehensive change to the code and there is always a risk that it won’t work as you hoped. By making the changes in a new branch, you can commit your changes without messing up the main branch. Plus, you can revert back to the main branch if the changes don’t work out. Making new branches in Git is quick and easy, so it’s good to get into the habit.

➤ Create a new file using the Swift File template. Name it Search.

➤ Change the contents of Search.swift to:

import Foundation

class Search {
  var searchResults: [SearchResult] = []
  var hasSearched = false
  var isLoading = false
	
  private var dataTask: URLSessionDataTask?

  func performSearch(for text: String, category: Int) {
    print("Searching...")
  }
}

You’ve given this class three internal properties, one private property, and a method. This stuff should look familiar because it comes straight from SearchViewController.

You’ll be removing code from that class and putting it into this new Search class.

The performSearch(for:category:) method doesn’t do much yet but that’s OK. First I want you to make SearchViewController work with this new Search object and when it compiles without errors, you will move all the logic over. Baby steps!

Move code over

Let’s make the changes to SearchViewController.swift. Xcode will probably give a bunch of errors and warnings while you’re making these changes, but it will all work out in the end.

➤ In SearchViewController.swift, remove the declarations for the following properties:

var searchResults: [SearchResult] = []
var hasSearched = false
var isLoading = false
var dataTask: URLSessionDataTask?

And replace them with this one:

private let search = Search()

The new Search object not only describes the state and results of the search, it will also encapsulate all the logic for talking to the iTunes web service. You can now remove a lot of code from the view controller.

➤ Move the following methods over to Search.swift:

• iTunesURL(searchText:category:)
• parse(data:)

➤ Make these methods private. They are only important to Search itself, not to any other classes from the app, so it’s good to “hide” them.

➤ Back in SearchViewController.swift, replace the performSearch() method with the following (Tip: set aside the old code in a temporary file because you’ll need it again later).

func performSearch() {
  search.performSearch(
    for: searchBar.text!, 
    category: segmentedControl.selectedSegmentIndex)
  
  tableView.reloadData()
  searchBar.resignFirstResponder()
}

This simply makes the Search object do all the work. Of course, you still reload the table view — to show the activity spinner — and hide the keyboard.

There are a few places in the code that still use the old searchResults array even though that no longer exists. You should change them to use the searchResults property from the Search object instead. Likewise for hasSearched and isLoading.

➤ For example, change tableView(_:numberOfRowsInSection:) to:

func tableView(
  _ tableView: UITableView, 
  numberOfRowsInSection section: Int
) -> Int {
  if search.isLoading {
    return 1  // Loading...
  } else if !search.hasSearched {
    return 0  // Not searched yet
  } else if search.searchResults.count == 0 {
    return 1  // Nothing Found
  } else {
    return search.searchResults.count
  }
}

Similar to the above, find the other places in code where the relevant properties have moved and make the necessary changes. If you aren’t sure of where to make the changes, look for Xcode errors — for this step, once you make all the changes correctly, the code will compile again without any errors.

➤ In showLandscape(with:), change the line that sets the searchResults property on the new view controller from:

controller.searchResults = search.searchResults

To:

controller.search = search

This line will give an error after you make the change, but you’ll fix that next.

The LandscapeViewController still has a property for a searchResults array so you have to change that to use the Search object as well.

➤ In LandscapeViewController.swift, remove the searchResults instance variable and replace it with:

var search: Search!

➤ In viewWillLayoutSubviews(), change the call to tileButtons() into:

tileButtons(search.searchResults)

OK, that’s the first round of changes. Build the app to make sure there are no compiler errors.

Add the search logic back in

The app itself doesn’t do much anymore because you removed all the searching logic. So let’s put that back in.

➤ In Search.swift, replace performSearch(for:category:) with the following (you can use that temporary file from earlier, but be careful to make the proper changes):

func performSearch(for text: String, category: Int) {
  if !text.isEmpty {
    dataTask?.cancel()

    isLoading = true
    hasSearched = true
    searchResults = []

    let url = iTunesURL(searchText: text, category: category)
    
    let session = URLSession.shared
    dataTask = session.dataTask(with: url) {
      data, response, error in
      // Was the search cancelled?
      if let error = error as NSError?, error.code == -999 {
        return
      }

      if let httpResponse = response as? HTTPURLResponse, 
        httpResponse.statusCode == 200, let data = data {
        self.searchResults = self.parse(data: data)
        self.searchResults.sort(by: <)

        print("Success!")
        self.isLoading = false
        return
      }

      print("Failure! \(response!)")
      self.hasSearched = false
      self.isLoading = false
    }
    dataTask?.resume()
  }
}

This is basically the same logic as before, except all the user interface code has been removed. The purpose of Search is just to perform a search, it should not do any UI stuff. That’s the job of the view controller.

➤ Run the app and search for something. When the search finishes, the Console shows a “Success!” message but the table view does not reload and the spinner keeps spinning for eternity.

The Search object currently has no way to tell the SearchViewController that it is done. You could solve this by making SearchViewController a delegate of the Search object, but for situations like these, closures are much more convenient.

The SearchComplete closure

Let’s create your own closure!

➤ Add the following line to Search.swift, above the class line:

typealias SearchComplete = (Bool) -> Void

The typealias declaration allows you to create a more convenient name for a data type, in order to save some keystrokes and to make the code more readable.

Here, you declare a type for your own closure, named SearchComplete. This is a closure that returns no value (it is Void) and takes one parameter, a Bool. If you think this syntax is weird, then I’m right there with you, but that’s the way it is.

From now on, you can use the name SearchComplete to refer to a closure that takes a Bool parameter and returns no value.

Closure types

Whenever you see a -> in a type definition, the type is intended for a closure, function, or method.

Swift treats these three things as mostly interchangeable. Closures, functions, and methods are all blocks of source code that possibly take parameters and return a value. The difference is that a function is really just a closure with a name, and a method is a function that lives inside an object.

Some examples of closure types:

() -> () is a closure that takes no parameters and returns no value.

Void -> Void is the same as the previous example – Void and () mean the same thing.

(Int) -> Bool is a closure that takes one parameter, an Int, and returns a Bool.

Int -> Bool is the same as the above. If there is only one parameter, you can leave out the parentheses.

(Int, String) -> Bool is a closure taking two parameters, an Int and a String, and returning a Bool.

(Int, String) -> Bool? as above, but now returns an optional Bool value.

(Int) -> (Int) -> Int is a closure that returns another closure that returns an Int. Freaky! Swift treats closures like any other type of object, so you can also pass them as parameters and return them from functions.

➤ Make the following changes to performSearch(for:category:):

func performSearch(
  for text: String, 
  category: Int, 
  completion: @escaping SearchComplete) {      // new
  if !text.isEmpty {
    . . .
    dataTask = session.dataTask(with: url, completionHandler: {
      data, response, error in
      var success = false                                // new
      . . .
      if let httpResponse = response as? . . . {
        . . .            
        self.isLoading = false
        success = true                     // instead of return
      }
      
      if !success {                                      // new
        self.hasSearched = false
        self.isLoading = false
      }                                                  // new
      // New code block - add the next three lines
      DispatchQueue.main.async {                         
        completion(success)
      }	
    })
    dataTask?.resume()
  }
}

You’ve added a third parameter named completion that is of type SearchComplete. Whoever calls performSearch(for:category:completion:) can now supply their own closure, and the method will execute the code that is inside that closure when the search completes.

Note: The @escaping annotation is necessary for closures that are not used immediately. It tells Swift that this closure may need to capture variables such as self and keep them around for a little while until the closure can finally be executed, in this case, when the search is done.

Instead of returning early from the closure upon success, you now set the success variable to true replacing the return statement. The value of success is used for the Bool parameter of the completion closure, as you can see inside the call to DispatchQueue.main.async at the bottom.

To perform the code from the closure, you simply call it as you’d call any function or method: closureName(parameters). You call completion(true) upon success and completion(false) upon failure. This is done so that the SearchViewController can reload its table view or, in the case of an error, show an alert view.

➤ In SearchViewController.swift, replace performSearch() with:

func performSearch() {
  search.performSearch(
    for: searchBar.text!, 
    category: segmentedControl.selectedSegmentIndex) { success in
      if !success {
        self.showNetworkError()
      }    
      self.tableView.reloadData()
    }
  
  tableView.reloadData()
  searchBar.resignFirstResponder()
}

You now pass a closure – as a trailing closure – to performSearch(for:category:completion:). The code in this closure gets called after the search completes, with the success parameter being either true or false. A lot simpler than making a delegate, right? The closure is always called on the main thread, so it’s safe to use UI code here.

➤ Run the app. You should be able to search again.

That’s the first part of this refactoring complete. You’ve extracted the relevant code for searching out of the SearchViewController and placed it into its own object, Search. The view controller now only does view-related things, which is exactly how it is supposed to work.

➤ You’ve made quite a few extensive changes, so it’s a good idea to commit.

Improve the categories

The idea behind Swift’s strong typing is that the data type of a variable should be as descriptive as possible. Right now, the category to search for is represented by a number, 0 to 3, but is that the best way to describe a category to your program?

If you see the number 3, does that mean “e-book” to you? It could be anything… And what if you use 4 or 99 or -1, what would that mean? These are all valid values for an Int but not for a category. The only reason the category is currently an Int is because segmentedControl.selectedSegmentIndex is an Int.

Represent the category as an enum

There are only four possible search categories, so this sounds like a job for an enum!

➤ Add the following to Search.swift, inside the class brackets:

enum Category: Int {
  case all = 0
  case music = 1
  case software = 2
  case ebooks = 3
}

This creates a new enumeration type named Category with four possible values. Each of these has a numeric value associated with it, called the raw value.

Contrast this with the AnimationStyle enum you made before:

enum AnimationStyle {
  case slide
  case fade
}

That enum does not associate numbers with its values — it doesn’t say : Int behind the enum name. For AnimationStyle it doesn’t matter that slide is really number 0 and fade is number 1, or whatever the values might be. All you care about is that a variable of type AnimationStyle can either be .slide or .fade, a numeric value is not important.

For the Category enum, however, you want to connect its four values to the four possible indices of the Segmented Control. If segment 3 is selected, you want this to correspond to .ebooks. That’s why the items from the Category enum have associated numbers.

Use the Category enum

➤ Change the method signature of performSearch(for:category:completion:) to use this new type:

func performSearch(
  for text: String, 
  category: Category,
  completion: @escaping SearchComplete) {

The category parameter is no longer an Int. It is not possible to pass it the value 4 or 99 or -1 anymore. It must always be one of the values from the Category enum. This reduces a potential source of bugs and it has made the program more expressive. Whenever you have a limited list of possible values that can be turned into an enum, it’s worth doing!

➤ Also change iTunesURL(searchText:category:) because that also assumed category would be an Int:

private func iTunesURL(searchText: String, category: Category) -> URL {
  let kind: String
  switch category {
  case .all: kind = ""
  case .music: kind = "musicTrack"
  case .software: kind = "software"
  case .ebooks: kind = "ebook"
  }
  
  let encodedText = . . .

The switch now looks at the various cases from the Category enum instead of the numbers 0 to 3. Note that the default case is no longer needed because the category parameter cannot have any other values.

This code works, but to be honest I’m not entirely happy with it. I’ve said before that any logic that is related to an object should be an integral part of that object. In other words, an object should do as much as it can itself.

Converting the category into a “kind” string that goes into the iTunes URL is a good example. That sounds like something the Category enum itself could do.

Swift enums can have their own methods and properties. So, let’s take advantage of that and improve the code even more.

➤ Add the type property to the Category enum:

enum Category: Int {
  case all = 0
  case music = 1
  case software = 2
  case ebooks = 3

  var type: String {
    switch self {
    case .all: return ""
    case .music: return "musicTrack"
    case .software: return "software"
    case .ebooks: return "ebook"
    }
  }
}

Swift enums cannot have instance variables, only computed properties. type has the exact same switch statement that you just saw, except that it switches on self, the current value of the enumeration object.

➤ In iTunesURL(searchText:category:) you can now simply write:

private func iTunesURL(searchText: String, category: Category) -> URL {
  let kind = category.type
  let encodedText = . . .

That’s a lot cleaner. Everything that has to do with categories now lives inside its own enum, Category.

Convert an Int to Category

You still need to tell SearchViewController about this, because it needs to convert the selected segment index into a proper Category value.

➤ In SearchViewController.swift, change the first part of performSearch() to:

func performSearch() {
  if let category = Search.Category(
    rawValue: segmentedControl.selectedSegmentIndex) {
    search.performSearch(
      for: searchBar.text!, 
      category: category) { success in
       . . .
    }
    . . .
  }
}

To convert the Int value from selectedSegmentIndex to an item from the Category enum, you use the built-in init(rawValue:) method. This may fail — for example, when you pass in a number that isn’t covered by one of Category’s cases, i.e. anything that is outside the range 0 to 3. That’s why init(rawValue:) returns an optional that needs to be unwrapped with if let before you can use it.

Note: Because you placed the Category enum inside the Search class, its full name is Search.Category. In other words, Category lives inside the Search namespace. It makes sense to bundle up these two things because they are so closely related.

➤ Build and run to see if the different categories still work.

Enums with associated values

Enums are pretty useful for restricting something to a limited range of possibilities, like what you did with the search categories. But they are even more powerful than you might have expected, as you’ll find out…

Like all objects, the Search object has a certain amount of state. For Search, this is determined by its isLoading, hasSearched, and searchResults variables.

These three variables describe four possible states:

The Search object is in only one of these states at a time, and when it changes from one state to another, there is a corresponding change in the app’s UI. For example, upon a change from “searching” to “have results”, the app hides the activity spinner and loads the results into the table view.

The problem is that this state is scattered across three different variables. It’s tricky to see what the current state is just by looking at these variables.

Consolidate search state

You can improve upon things by giving Search an explicit state variable. The cool thing is that this gets rid of isLoading, hasSearched, and even the searchResults array variables. Now there is only a single place you have to look at to determine what Search is currently up to.

➤ In Search.swift, remove the following instance variables:

var searchResults: [SearchResult] = []
var hasSearched = false
var isLoading = false

➤ In their place, add the following enum, which goes inside the class again:

enum State {
  case notSearchedYet
  case loading
  case noResults
  case results([SearchResult])
}

This enumeration has a case for each of the four states listed above. It does not need raw values, so the cases don’t have numbers — do note that the state .notSearchedYet is also used for when there is an error.

The .results case is special: it has an associated value — an array of SearchResult objects.

This array is only important when the search is successful. In all the other cases, there are no search results and the array is empty — see the state table above. By making it an associated value, you’ll only have access to this array when Search is in the .results state. In the other states, the array simply does not exist.

Use the new state enum

Let’s see how this works.

➤ First add a new instance variable:

private(set) var state: State = .notSearchedYet

This keeps track of Search’s current state. Its initial value is .notSearchedYet — obviously no search has happened yet when the Search object is first constructed.

This variable is private, but only half so. It’s not unreasonable for other objects to want to ask Search what its current state is. In fact, the app won’t work unless you allow this.

But you don’t want those other objects to be able to change the value of state; they are only allowed to read the state value. With private(set) you tell Swift that reading is OK for other objects, but assigning (or setting) new values to this variable may only happen inside the Search class.

➤ Change performSearch(for:category:completion:) to use this new variable:

func performSearch(
  for text: String, 
  category: Category,
  completion: @escaping SearchComplete
) {
  if !text.isEmpty {
    dataTask?.cancel()
    // Remove the next 3 lines and replace with the following
    state = .loading                                
    . . .
    dataTask = session.dataTask(with: url) {
      data, response, error in
      
      var newState = State.notSearchedYet           // add this
      . . .      
      if let httpResponse = response . . . {
        // Replace all code within this if block with following
        var searchResults = self.parse(data: data)
        if searchResults.isEmpty {
          newState = .noResults
        } else {
          searchResults.sort(by: <)
          newState = .results(searchResults)
        }
        success = true
      }
      // Remove "if !success" block
      DispatchQueue.main.async {
        self.state = newState                        // add this
        completion(success)
      }
    }
    dataTask?.resume()
  }
}

Instead of the old variables isLoading, hasSearched, and searchResults, this code now only changes state.

Note: You don’t update state directly, but instead, use a new local variable newState. Then at the end, in the DispatchQueue.main.async block, you transfer the value of newState to self.state. The reason for doing this the long way round is that state must only be changed by the main thread, or it can lead to a nasty and unpredictable bug known as a race condition.

When you have multiple threads trying to use the same variable at the same time, the app may do unexpected things and crash. In our app, the main thread will try to use search.state to display the activity spinner in the table view — and that can happen at the same time as URLSession’s completion handler, which runs in a background thread. We have to make sure these two threads don’t get in each other’s way!

Here’s how the new logic works:

There is a lot that can go wrong between performing the network request and parsing the JSON. By setting newState to .notSearchedYet (which doubles as the error state) and success to false at the start of the completion handler, you assume the worst — always a good idea when doing network programming — unless there is evidence otherwise.

That evidence comes when the app is able to successfully parse the JSON and create an array of SearchResult objects. If the array is empty, newState becomes .noResults.

The interesting part is when the array is not empty. After sorting it like before, you do newState = .results(searchResults). This gives newState the value .results and also associates the array of SearchResult objects with it. You no longer need a separate instance variable to keep track of the array; the array object is intrinsically attached to the value of newState.

Finally, you copy the value of newState into self.state. As I mentioned, this needs to happen on the main thread to prevent race conditions.

Update other classes to use the state enum

That completes the changes in Search.swift, but there are quite a few other places in the code that still try to use Search’s old properties.

➤ In SearchViewController.swift, replace tableView(_:numberOfRowsInSection:) with:

func tableView(
  _ tableView: UITableView, 
  numberOfRowsInSection section: Int
) -> Int {
  switch search.state {
  case .notSearchedYet:
    return 0
  case .loading:
    return 1
  case .noResults:
    return 1
  case .results(let list):
    return list.count
  }
}

This is pretty straightforward — instead of trying to make sense out of the separate isLoading, hasSearched, and searchResults variables, this simply looks at the value from search.state. The switch statement is ideal for situations like this.

The .results case requires a bit more explanation. Because .results has an array of SearchResult objects associated with it, you can bind this array to a temporary variable, list, and then use that variable inside the case to read how many items are in the array. That’s how you make use of the associated value. This pattern, using a switch statement to look at state, is going to become very common in your code.

➤ Replace tableView(_:cellForRowAt:) with:

func tableView(
  _ tableView: UITableView,
  cellForRowAt indexPath: IndexPath
) -> UITableViewCell {
  switch search.state {
  case .notSearchedYet:
    fatalError("Should never get here")
  
  case .loading:
    let cell = tableView.dequeueReusableCell(
      withIdentifier: TableView.CellIdentifiers.loadingCell, 
      for: indexPath)
    
    let spinner = cell.viewWithTag(100) as! UIActivityIndicatorView
    spinner.startAnimating()
    return cell
  
  case .noResults:
    return tableView.dequeueReusableCell(
      withIdentifier: TableView.CellIdentifiers.nothingFoundCell,
      for: indexPath)
    
  case .results(let list):
    let cell = tableView.dequeueReusableCell(
      withIdentifier: TableView.CellIdentifiers.searchResultCell,
      for: indexPath) as! SearchResultCell
    
    let searchResult = list[indexPath.row]
    cell.configure(for: searchResult)
    return cell
  }
}

The same thing happens here. The various if statements have been replaced by a switch and case statements for the four possibilities.

Note that numberOfRowsInSection returns 0 for .notSearchedYet and no cells will ever be asked for. But because a switch must always be exhaustive, you also have to include a case for .notSearchedYet in cellForRowAt. Since it would be a bug if the code ever got there, you can use the built-in fatalError() function to help catch such a situation.

➤ Next up is tableView(_:willSelectRowAt:):

func tableView(
  _ tableView: UITableView, 
  willSelectRowAt indexPath: IndexPath
) -> IndexPath? {
  switch search.state {
  case .notSearchedYet, .loading, .noResults:
    return nil
  case .results:
    return indexPath
  }
}

It’s only possible to tap on rows when the state is .results. So for all the other cases, this method returns nil. And for the .results case, you don’t need to bind the results array because you’re not using it for anything here.

➤ And finally, change prepare(for:sender:) to:

override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
  if segue.identifier == "ShowDetail" {
    if case .results(let list) = search.state {
      let detailViewController = segue.destination as! DetailViewController
      let indexPath = sender as! IndexPath
      let searchResult = list[indexPath.row]
      detailViewController.searchResult = searchResult
    }
  }
}

Here you only care about the .results case, so writing an entire switch statement is a bit much. For situations like this, you can use the special if case statement to look at a single case.

There is one more change to make in LandscapeViewController.swift.

➤ Change the if firstTime block in viewWillLayoutSubviews() to:

if firstTime {
  firstTime = false
  
  switch search.state {
  case .notSearchedYet, .loading, .noResults:
    break
  case .results(let list):
    tileButtons(list)
  }
}

This uses the same pattern as before. If the state is .results, it binds the array of SearchResult objects to the temporary constant list and passes it along to tileButtons(). The reason you don’t use a if case condition here is because you’ll be adding additional code to the other cases soon. But, because these cases are currently empty, they must contain a break statement.

However, when multiple cases have the same action, you can combine them in a single case statement as you see above.

➤ Build and run to see if the app still works — it should!

I think enums with associated values are one of the most exciting features of Swift. Here you used them to simplify the way the Search state is expressed. No doubt you’ll find many other great uses for them in your own apps!

➤ This is a good time to commit your changes.

Spin me right round

If you rotate to landscape while the search is still taking place, the app really ought to show an animated spinner to let the user know that an action is taking place. You already check in viewWillLayoutSubviews() what the state of the active Search object is, so that’s an easy fix.

Show an activity indicator in landscape mode

➤ In LandscapeViewController.swift, add a new method to display an activity indicator:

private func showSpinner() {
  let spinner = UIActivityIndicatorView(style: .large)
  spinner.center = CGPoint(
    x: scrollView.bounds.midX + 0.5, 
    y: scrollView.bounds.midY + 0.5)
  spinner.tag = 1000
  view.addSubview(spinner)
  spinner.startAnimating()
}

This creates a new UIActivityIndicatorView object, puts it in the center of the screen, and starts animating it. You give the spinner the tag 1000, so you can easily remove it from the screen once the search is done.

➤ In viewWillLayoutSubviews() change the .loading case in the switch statement to call this new method – you’ll have to move the loading case out of the combined case line too:

case .loading:
  showSpinner()

➤ Run the app. After starting a search, quickly rotate the phone to landscape. You should now see a spinner:

Note: In the new method you add 0.5 to the spinner’s center position. This kind of spinner is 37 points wide and high, which is not an even number. If you were to place the center of this view at the exact center of the screen at (284, 160) then it would extend 18.5 points to either end. The top-left corner of that spinner will be at coordinates (265.5, 141.5), making it look all blurry.

It’s best to avoid placing objects at fractional coordinates. By adding 0.5 to both the X and Y position, the spinner is placed at (266, 142) and everything looks sharp. Pay attention to this when working with the center property and objects that have odd widths or heights.

Hide the landscape spinner when results are found

This is all great, but the spinner doesn’t disappear when the actual search results are received. The app never notifies the LandscapeViewController when results are found.

There is a variety of ways you can choose to tell the LandscapeViewController that the search results have come in, but let’s keep it simple.

➤ In LandscapeViewController.swift, add these two new methods:

// MARK: - Helper Methods
func searchResultsReceived() {
  hideSpinner()
  
  switch search.state {
  case .notSearchedYet, .loading, .noResults:
    break
  case .results(let list):
    tileButtons(list)
  }
}

private func hideSpinner() {
  view.viewWithTag(1000)?.removeFromSuperview()
}

The private hideSpinner() method looks for the view with tag 1000 — the activity spinner — and then tells that view to remove itself from the screen.

You could have kept a reference to the spinner and used that, but for a simple situation such as this you might as well use a tag.

Because no one else has any strong references to the UIActivityIndicatorView, this instance will be deallocated. Note that you have to use optional chaining because viewWithTag() can potentially return nil.

The searchResultsReceived() method should be called from somewhere, of course, and that somewhere is the SearchViewController.

➤ In SearchViewController.swift’s performSearch() method, add the following line into the closure, below self.tableView.reloadData():

self.landscapeVC?.searchResultsReceived()

The sequence of events here is quite interesting. When the search begins there is no LandscapeViewController object yet because the only way to start a search is from portrait mode.

But by the time the closure is invoked, the device may have rotated and if that happened self.landscapeVC will contain a valid reference.

Upon rotation, you also gave the new LandscapeViewController a reference to the active Search object. Now you just have to tell it that search results are available so it can create the buttons and fill them up with images.

Of course, if you’re still in portrait mode by the time the search completes, then self.landscapeVC is nil and the call to searchResultsReceived() will simply be ignored due to the optional chaining — you could have used if let here to unwrap the value of self.landscapeVC, but optional chaining has the same effect and is shorter to write.

➤ Try it out. That works pretty well, eh?

Exercise. Verify that network errors are also handled correctly when the app is in landscape orientation. Find a way to create, or fake, a network error and see what happens in landscape mode. Hint: if you don’t want to use the Network Link Conditioner, the sleep(5) function will put your app to sleep for 5 seconds. Put that in the completion handler to give yourself some time to flip the device around.

Nothing found

You’re not done yet. If there are no matches found, you should also tell the user about this if they’re in landscape mode.

➤ First, add the following method to LandscapeViewController.swift:

private func showNothingFoundLabel() {
  let label = UILabel(frame: CGRect.zero)
  label.text = "Nothing Found"
  label.textColor = UIColor.label
  label.backgroundColor = UIColor.clear
  
  label.sizeToFit()
  
  var rect = label.frame
  rect.size.width = ceil(rect.size.width / 2) * 2    // make even
  rect.size.height = ceil(rect.size.height / 2) * 2  // make even
  label.frame = rect
  
  label.center = CGPoint(
    x: scrollView.bounds.midX, 
    y: scrollView.bounds.midY)
  view.addSubview(label)
}

You first create a UILabel object and give it text and a color — note that the color is the system label color so that the text would display correctly in either appearance. The backgroundColor property is set to UIColor.clear to make the label transparent.

The call to sizeToFit() tells the label to resize itself to the optimal size. You could have given the label a frame that was big enough to begin with, but I find this just as easy. This also helps when you’re translating the app to a different language, in which case you may not know beforehand how large the label needs to be.

The only trouble is that you want to center the label in the view and as you saw before, that gets tricky when the width or height are odd — something you don’t necessarily know in advance. So here you use a little trick to always force the dimensions of the label to be even numbers:

width = ceil(width/2) * 2

If you divide a number such as 11 by 2 you get 5.5. The ceil() function rounds up 5.5 to make 6, and then you multiply by 2 to get a final value of 12. This formula always gives you the next even number if the original is odd. You only need to do this because these values have type CGFloat. If they were integers, you wouldn’t have to worry about fractional parts.

Note: Because you’re not using a hardcoded number such as 480 or 568 but scrollView.bounds to determine the width of the screen, the code to center the label works correctly on all screen sizes.

➤ Inside the switch statement in viewWillLayoutSubviews(), call the new method from the case for .noResults:

case .noResults:
  showNothingFoundLabel()

➤ Run the app and search for something ridiculous (ewdasuq3sadf843 will do). When the search is done, flip to landscape.

It doesn’t work properly yet if you flip to landscape while the search is taking place. Of course, you also need to put some logic in searchResultsReceived().

➤ Change the switch statement in that method to:

switch search.state {
case .notSearchedYet, .loading:
  break
case .noResults:
  showNothingFoundLabel()
case .results(let list):
  tileButtons(list)
}

Now you should have all your bases covered.

The Detail pop-up

The landscape view is that much more functional after all the refactoring and changes. But there’s still one more thing left to do. The landscape search results are not buttons for nothing.

The app should show the Detail pop-up when you tap an item.

This is fairly easy to achieve. When adding the buttons you can give them a target-action — a method to call when the Touch Up Inside event is received. Just like in Interface Builder, except now you hook up the event to the action method programmatically.

Show the Detail pop-up

➤ First, still in LandscapeViewController.swift add the method to be called when a button is tapped:

@objc func buttonPressed(_ sender: UIButton) {
  performSegue(withIdentifier: "ShowDetail", sender: sender)
}

Even though this is an action method, you didn’t declare it as @IBAction. That is only necessary when you want to connect the method to something in Interface Builder. Here you make the connection via code, so you can skip the @IBAction annotation.

Also note that the method has the @objc attribute — as you learnt previously with MyLocations, you need to tag any method that is identified via a #selector with the @objc attribute. So, that would seem to indicate that you’ll be calling this new method using a #selector, right?

Pressing the button simply triggers a segue, and you’ll get to the segue part in a moment. But first, you should hook up the buttons to the above method.

➤ Add the following two lines to the button creation code in tileButtons():

button.tag = 2000 + index
button.addTarget(
  self, 
  action: #selector(buttonPressed), 
  for: .touchUpInside)

First you give the button a tag, so you know to which index in the .results array this button corresponds. That’s needed in order to pass the correct SearchResult object to the Detail pop-up.

Also, if you replaced the index variable in the for loop earlier with a wildcard because of the Xcode compiler warning, this would be the time to revert that change.

Tip: You added 2000 to the index because tag 0 is used on all views by default, so asking for a view with tag 0 might actually return a view that you didn’t expect. To avoid this kind of confusion, you simply start counting from 2000.

You also tell the button it should call the buttonPressed() method when it gets tapped.

➤ Next, add the prepare(for:sender:) method to handle the segue:

// MARK: - Navigation 
override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
  if segue.identifier == "ShowDetail" {
    if case .results(let list) = search.state {
      let detailViewController = segue.destination as! DetailViewController
      let searchResult = list[(sender as! UIButton).tag - 2000]
      detailViewController.searchResult = searchResult
    }
  }
}

This is almost identical to prepare(for:sender:) from SearchViewController, except now you don’t get the index of the SearchResult object from an index-path, but from the button’s tag minus 2000.

Of course, none of this will work unless you actually have a segue in the storyboard.

➤ Go to the Landscape scene in the storyboard and Control-drag from the yellow circle at the top to the Detail View Controller. Make it a Present Modally segue with the identifier set to ShowDetail. The storyboard should look like this now:

➤ Run the app and check it out. It probably looks like this:

Fix the detail pop-up

Hmm … that’s not quite what you were expecting, was it?

Exercise: Do you know what went wrong?

Answer: When you removed the width constraint on the pop-up in order to support really large fonts after adding Dynamic Type support, you were only dealing with portrait mode. In portrait mode, generally, the even at its widest, the pop-up would look fine. But not in landscape mode …

There are several ways to fix this:

1. Add the width constraint back so that the pop-up always displays at a reasonable size whether in portrait or landscape mode.
2. Set up separate constraints for landscape mode for the pop-up so that it isn’t so wide.

While option #1 is easier, option #2 will make the app function better for each device orientation. So let’s go with option #2.

You could, of course, add outlets for the relevant constraints and change them depending on the device orientation. But that’s something that you’re already familiar with. Let’s learn a different way that teaches you how to set up constraints based on traits :]

➤ Open the storyboard, select Pop-up View, go to the Size inspector, select the Leading to: constraint to the Safe Area and double click it to get the constraint editor:

➤ Click the + (plus) button next to Constant to be able to add a custom Constant value based on a few factors available from the new popup which opens:

You can add variations based on the size classes you already learnt about when you set up the landscape view. The new dialog is pre-configured for the currently selected device and orientation that you selected via the View as: panel.

So, you would be adding a new variation for an iPhone SE in landscape mode at this point if you went with the default values.

➤ Click Add Variation.

You should now get a new value under Constant for the specific size class variation you requested:

➤ Enter 150 as the new value – notice how the leading constraint changes on the canvas for your pop-up view.

➤ Similarly set up a new variation value of 150 for the trailing constraint too.

Your pop-up view looks much more compact in Interface Builder now. But what about other size classes?

➤ Use the View as: panel to switch your preview to a larger device like the iPhone 11 Pro Max.

You’ll notice that the pop-up view is again too wide. This is because the iPhone 11 Pro Max (and similar devices) have a Regular size class for the height when in landscape mode.

➤ Add two new variations — one for leading and another for trailing — for this size class too. Feel free to adjust the spacing as you see fit if you think a value of 150 is not enough.

Now, your detail pop-up looks much better in landscape:

Hide the pop-up on rotation

Cool! But what happens when you rotate back to portrait with a Detail pop-up showing? Unfortunately, it sticks around. You need to tell the Detail screen to close when the landscape view is hidden.

➤ In SearchViewController.swift, in hideLandscape(with:), add the following lines to the animate(alongsideTransition:) animation closure:

if self.presentedViewController != nil {
  self.dismiss(animated: true, completion: nil)
}

In the Console output you should see that the DetailViewController is properly deallocated when you rotate back to portrait.

➤ If you’re happy with the way the code works, then let’s commit it. If you also made a branch, then merge it back into the main branch.

You can find the project files for this chapter under 40-Refactoring in the Source Code folder.