Displaying JSON data from a server in a UICollectionView

Question

This code pulls JSON from a server and delivers some data like imageURL, titleString and videoURL. Then it places them in a UICollectionView and displays them.

import UIKit
import Alamofire

class vcWatch: UIViewController, UICollectionViewDelegate, UICollectionViewDataSource {

@IBOutlet weak var myActivityIndicator: UIActivityIndicatorView!
@IBOutlet weak var myCollectionView: UICollectionView!

var images:[String] = []
var videos:[String] = []
var lableTitles:[String] = []

let link = "http://alifetouched.com/lib/videos.json.php"

override func viewDidLoad() {
    super.viewDidLoad()

    myActivityIndicator.isHidden = false
    myActivityIndicator.startAnimating()
    loadImages()
}

func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
    return images.count
}

func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {

    let cell:cvCell = collectionView.dequeueReusableCell(withReuseIdentifier: "VideoCell", for: indexPath) as! cvCell

    // Videos
    let requestURL = URL(string:self.videos[indexPath.row])
    let request = URLRequest(url: requestURL!)

    // Images
    let imageString = self.images[indexPath.row]
    let imageUrl = NSURL(string: imageString)
    let imageData = NSData(contentsOf: imageUrl! as URL)

    cell.videoCell.loadRequest(request)
    cell.labelCell.text = self.lableTitles[indexPath.row]

    if(imageData != nil){
        cell.imageCell.image = UIImage(data: imageData! as Data)
    }

    return cell
}

func collectionView(_ collectionView: UICollectionView, didDeselectItemAt indexPath: IndexPath){
    print("User Tapped: \(indexPath.row)")
}

func loadImages() {

    Alamofire.request(link)
        .validate()
        .responseJSON { (response) in

            guard response.result.isSuccess else {
                print("Error with response: \(response.result.error)")
                return
            }

            guard let dict = response.result.value as? Dictionary <String,AnyObject> else {
                print("Error with dictionary: \(response.result.error)")
                return
            }

            guard let dictData = dict["VideoJSON"] as? [Dictionary <String,AnyObject>] else {
                print("Error with dictionary data: \(response.result.error)")
                return
            }

            for data in dictData {
                self.videos.append(data["media_url"] as! String)
                self.images.append(data["icon_url"] as! String)
                self.lableTitles.append(data["title"] as! String)
            }

            self.myCollectionView.reloadData()
            self.myActivityIndicator.stopAnimating()
            self.myActivityIndicator.isHidden = true

            return

    }
}

Answer 1

I will address a few points that are the most important in my opinion.

1. Be consistent.

   You're using URL but right after that you're also using NSURL. You're using NSData but later have to cast it to Data. Etc.

   The right way would be, instead, to only use URL and Data, and forget the other types.

   let requestURL = URL(string:self.videos[indexPath.row])
   let request = URLRequest(url: requestURL!)
   let imageUrl = URL(string: imageString)
   let imageData = try! Data(contentsOf: imageUrl!)
   
   cell.imageCell.image = UIImage(data: imageData)
   

2. Don't force unwrap.

   You should not force unwrap your Optionals but use safe unwrapping instead, with if let or guard let.

   A classic solution in your case is to have a ready-made placeholder image in your project - that way, if the download of an image fails, you can use the placeholder instead.

   if let imageUrl = URL(string: imageString),
       let imageData = try? Data(contentsOf: imageUrl)
   {
       cell.imageCell.image = UIImage(data: imageData) 
   } else {
       cell.imageCell.image = placeholder
   }
   

3. Download asynchronously

   You are downloading your list of images URLs with Alamofire but you're downloading the images themselves with NSData(contentsOf: imageUrl! as URL).

   The problem is that this method is synchronous and blocks the main thread. If the network is suddenly slow or stops working, your app will lag or freeze.

   Instead, download asynchronously with Alamofire, URLSession, etc.

   Since you already know how to use Alamofire, I'm showing it with URLSession as an alternative:

   if let imageUrl = URL(string: imageString) {
       URLSession.shared.dataTask(with: URLRequest(url: imageUrl)) { (data, response, error) in
           if let data = data {
               DispatchQueue.main.async {
                   cell.imageCell.image = UIImage(data: data)  
               }
           } else {
               DispatchQueue.main.async {
                   cell.imageCell.image = placeholder
               }
           }
       }.resume()
   }
   

   Note that because .dataTask is asynchronous, and because we have to do all UI operations on the main thread, we have to set the image using DispatchQueue.main.async once it's downloaded.

   This way, the UI won't freeze if there's network issues.

4. Style

   This one is rather subjective, but in Swift there's a few common style guides you should follow:

   Classes should start with an uppercase letter:

   class VCWatch
   
   class CVCell
   

   Type declaration should have a space after the colon:

   var images: [String] = []
   var videos: [String] = []
   var lableTitles: [String] = []
   

   Parenthesis are not necessary for a boolean condition:

   if imageData != nil {
       cell.imageCell.image = UIImage(data: imageData)
   }
   

   You can omit the type declaration when the compiler already knows the type:

   let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "VideoCell", for: indexPath) as! cvCell
   

   You can use short syntax for arrays and dictionaries:

   [String: AnyObject] // instead of Dictionary<String,AnyObject>