patternswiftMinor
UICollectionView image gallery app
Viewed 0 times
uicollectionviewimagegalleryapp
Problem
I built an app that allows users to add and remove images in a collection view. I want this to be useful online so I save the images into a database and retrieve them every time. I'm looking for constructive criticism on how I solved this problem. It is in swift. The code works just fine, but I notice some lag sometimes, and it may be because of the way I designed the software. This is why I'm looking for criticism.
ViewGallery.swift
```
import UIKit
import Firebase
import DKImagePickerController
class ViewGallery: UIViewController, UICollectionViewDataSource, UICollectionViewDelegate, UICollectionViewDelegateFlowLayout {
let db = Firebase(url: "mydatabaseurl")
@IBOutlet var collectionView: UICollectionView!
var imageDatas: [String] = []
var selectedItems: [NSIndexPath] = []
var keys: [AnyObject] = []
override func viewDidLoad() {
super.viewDidLoad()
db.childByAppendingPath("Images").observeEventType(.Value, withBlock: {
snapshot in
let images = snapshot.value as! NSDictionary
self.keys = images.allKeys
self.imageDatas = []
for key in self.keys {
self.imageDatas.append(images[key as! NSString as String]!["image"] as! String)
}
self.selectedItems = []
self.collectionView.reloadData()
})
}
@IBAction func addPictures(sender: AnyObject) {
let pickerController = DKImagePickerController()
pickerController.didSelectAssets = { [unowned self] (assets: [DKAsset]) in
print("didSelectAssets")
for asset in assets {
let imageData = asset.rawData?.base64EncodedStringWithOptions(.Encoding64CharacterLineLength)
self.saveImageStringToDatabase(imageData!)
}
}
self.presentViewController(pickerController, animated: true, completion: nil)
}
@IBAction func removePicture(sender: AnyObject) {
for indexPath in selectedItems {
db.childByAppendingPath("Images").childByAppendingPath(self.keys[indexPath.item] as! String).removeValue()
}
}
func saveImageStringToDatabase(imageString: String) {
let
ViewGallery.swift
```
import UIKit
import Firebase
import DKImagePickerController
class ViewGallery: UIViewController, UICollectionViewDataSource, UICollectionViewDelegate, UICollectionViewDelegateFlowLayout {
let db = Firebase(url: "mydatabaseurl")
@IBOutlet var collectionView: UICollectionView!
var imageDatas: [String] = []
var selectedItems: [NSIndexPath] = []
var keys: [AnyObject] = []
override func viewDidLoad() {
super.viewDidLoad()
db.childByAppendingPath("Images").observeEventType(.Value, withBlock: {
snapshot in
let images = snapshot.value as! NSDictionary
self.keys = images.allKeys
self.imageDatas = []
for key in self.keys {
self.imageDatas.append(images[key as! NSString as String]!["image"] as! String)
}
self.selectedItems = []
self.collectionView.reloadData()
})
}
@IBAction func addPictures(sender: AnyObject) {
let pickerController = DKImagePickerController()
pickerController.didSelectAssets = { [unowned self] (assets: [DKAsset]) in
print("didSelectAssets")
for asset in assets {
let imageData = asset.rawData?.base64EncodedStringWithOptions(.Encoding64CharacterLineLength)
self.saveImageStringToDatabase(imageData!)
}
}
self.presentViewController(pickerController, animated: true, completion: nil)
}
@IBAction func removePicture(sender: AnyObject) {
for indexPath in selectedItems {
db.childByAppendingPath("Images").childByAppendingPath(self.keys[indexPath.item] as! String).removeValue()
}
}
func saveImageStringToDatabase(imageString: String) {
let
Solution
I am not familiar with the Firebase or DKImagePickerController framework, so I cannot say anything
about that. But there are some things that I noticed:
There is no need in Swift to reference properties via
(unless there is an ambiguity with a local variable, e.g. in an init
method). And you are not consistent here, e.g. both
and
This
can be simplified to
And in
there is no need to check for
is empty then
collection view displays no item at all and
will not be called.
If the order of the selected items is not relevant then you can
use a set instead of an array:
Adding or removing an index path then simplifies slightly to
A loop like
can be replaced by a mapping:
about that. But there are some things that I noticed:
There is no need in Swift to reference properties via
self(unless there is an ambiguity with a local variable, e.g. in an init
method). And you are not consistent here, e.g. both
self.selectedItemsand
selectedItems is used.This
func collectionView(collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
if imageDatas.count == 0 {
return 0
} else {
return imageDatas.count
}
}can be simplified to
func collectionView(collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
return imageDatas.count
}And in
func collectionView(collectionView: UICollectionView, cellForItemAtIndexPath indexPath: NSIndexPath) -> UICollectionViewCellthere is no need to check for
imageDatas.count == 0 (and return a dummy cell in that case). If the arrayis empty then
numberOfItemsInSection returns zero, i.e. thecollection view displays no item at all and
cellForItemAtIndexPathwill not be called.
If the order of the selected items is not relevant then you can
use a set instead of an array:
var selectedItems = Set()Adding or removing an index path then simplifies slightly to
if selectedItems.contains(indexPath) {
cell.backgroundColor = UIColor.clearColor()
selectedItems.remove(indexPath)
} else {
cell.backgroundColor = UIColor.redColor()
selectedItems.insert(indexPath)
}A loop like
self.imageDatas = []
for key in self.keys {
self.imageDatas.append(...)
}can be replaced by a mapping:
self.imageDatas = self.keys.map { key in ... }Code Snippets
func collectionView(collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
if imageDatas.count == 0 {
return 0
} else {
return imageDatas.count
}
}func collectionView(collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
return imageDatas.count
}func collectionView(collectionView: UICollectionView, cellForItemAtIndexPath indexPath: NSIndexPath) -> UICollectionViewCellvar selectedItems = Set<NSIndexPath>()if selectedItems.contains(indexPath) {
cell.backgroundColor = UIColor.clearColor()
selectedItems.remove(indexPath)
} else {
cell.backgroundColor = UIColor.redColor()
selectedItems.insert(indexPath)
}Context
StackExchange Code Review Q#113965, answer score: 4
Revisions (0)
No revisions yet.