patternswiftMinor
Displaying JSON data from a server in a UICollectionView
Viewed 0 times
uicollectionviewserverdisplayingjsonfromdata
Problem
This code pulls JSON from a server and delivers some data like
```
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
}
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
}
Solution
I will address a few points that are the most important in my opinion.
-
Be consistent.
You're using
The right way would be, instead, to only use
-
Don't force unwrap.
You should not force unwrap your Optionals but use safe unwrapping instead, with
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.
-
Download asynchronously
You are downloading your list of images URLs with Alamofire but you're downloading the images themselves with
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
Note that because
This way, the UI won't freeze if there's network issues.
-
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:
Type declaration should have a space after the colon:
Parenthesis are not necessary for a boolean condition:
You can omit the type declaration when the compiler already knows the type:
You can use short syntax for arrays and dictionaries:
-
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)-
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
}-
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.
-
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 CVCellType 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! cvCellYou can use short syntax for arrays and dictionaries:
[String: AnyObject] // instead of DictionaryCode Snippets
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)if let imageUrl = URL(string: imageString),
let imageData = try? Data(contentsOf: imageUrl)
{
cell.imageCell.image = UIImage(data: imageData)
} else {
cell.imageCell.image = placeholder
}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()
}class VCWatch
class CVCellvar images: [String] = []
var videos: [String] = []
var lableTitles: [String] = []Context
StackExchange Code Review Q#144260, answer score: 4
Revisions (0)
No revisions yet.