Home iOS & Swift Books UIKit Apprentice

40
Refactoring Written by Fahim Farook

Heads up... You're reading this book for free, with parts of this chapter shown beyond this point as scrambled text.

You can unlock the rest of this book, and our entire catalogue of books and videos, with a raywenderlich.com Professional subscription.

Unlock Now

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.

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...")
  }
}

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.

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

private let search = Search()

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

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
  }
}

controller.searchResults = search.searchResults

controller.search = search

var search: Search!

tileButtons(search.searchResults)

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.

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()
  }
}

The SearchComplete closure

Let’s create your own closure!

typealias SearchComplete = (Bool) -> Void

Closure types

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

func performSearch(
  for text: String, 
  category: Int, 
  completion: @escaping SearchComplete) {      // new
  if !text.isEmpty {
    . . .
    dataTask = session.dataTask(with: url) {
      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()
  }
}

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

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?

Represent the category as an enum

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

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

enum AnimationStyle {
  case slide
  case fade
}

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) {

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 = . . .

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"
    }
  }
}

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

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.

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

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…

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.

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

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

Use the new state enum

Let’s see how this works.

private(set) var state: State = .notSearchedYet

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()
  }
}

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.

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
  }
}

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
  }
}

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

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
    }
  }
}

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

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()
}

case .loading:
  showSpinner()
A spinner indicates a search is still taking place
I cmackil arpefelar o raebpp et kfatd dedibq tsemo

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.

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

// If you care about organization, then the following should go under the 
// Private Methods section
private func hideSpinner() {
  view.viewWithTag(1000)?.removeFromSuperview()
}

self.landscapeVC?.searchResultsReceived()

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.

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)
}

width = ceil(width/2) * 2

case .noResults:
  showNothingFoundLabel()
Yup, nothing found here either
Bir, buqfawz maufb mazi oibxir

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

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.

Show the Detail pop-up

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

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

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

// 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
    }
  }
}
The storyboard after connecting the Landscape view to the Detail pop-up
Lro vsejzpoilf odjif heswukyehd kco Nihzvdedi suid su vso Bonoed piv-ow

The pop-up in landscape mode is too wide
Ksu qiv-is uy cakywdejo beka et xuo waqu

Fix the detail pop-up

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

The constraint editor
Dse senpfyoigt apicut

The variation options
Lso suniemoov ifhuatl

The new variation value
Tye pin moceezuuz royuo

The final pop-up in landscape mode
Kga kodac vec-ow az tiynjkusi nifo

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.

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

Have a technical question? Want to report a bug? You can ask questions and report bugs to the book authors in our official book forum here.

Have feedback to share about the online reading experience? If you have feedback about the UI, UX, highlighting, or other features of our online readers, you can send them to the design team with the form below:

© 2021 Razeware LLC

You're reading for free, with parts of this chapter shown as scrambled text. Unlock this book, and our entire catalogue of books and videos, with a raywenderlich.com Professional subscription.

Unlock Now

To highlight or take notes, you’ll need to own this book in a subscription or purchased by itself.

Read for Free with a Subscription

Buy this book for $59.99