HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

UICollectionView image gallery app

Submitted by: @import:stackexchange-codereview··
0
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

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 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.selectedItems
and 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) -> UICollectionViewCell


there is no need to check for imageDatas.count == 0 (and return a dummy cell in that case). If the array
is empty then numberOfItemsInSection returns zero, i.e. the
collection view displays no item at all and cellForItemAtIndexPath
will 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) -> UICollectionViewCell
var 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.