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

    }
}
up vote 4 down vote accepted

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>
    
  • 1
    Thank you so much for taking the time to be so detailed. I will start dissecting your solution and quickly implement it in the app. This is wonderful - thank you. – Dortoh Oct 30 '16 at 5:19

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.