iOS Swift 3 app that requests data from the Stack Exchange API

Question

This is my first Swift app, so I want to improve everything that I can about my code and know what I can do better. The app requests 10 developers from StackExchange API and displays them in a table with the possibility to click on one of them and see more details about him.

I was asked to achieve the following memory management caching:

1. If the data is in RAM, get it from there

2. Else, if it is on disk, get it from there

3. Else, make the API request and save the data afterwards

Data on disk should stay for 30 minutes.

I'm not sure if what I did with NSCache was good, I mean if I have the data in my viewController class I don't think I need to implement NSCache because it goes in ram anyway right? I wish to see what I can improve in this project.

ViewController code bellow and link to GitHub repo:

GitHub repo

import UIKit
import Alamofire
import Foundation


class ViewController: UIViewController,UITableViewDelegate,UITableViewDataSource{

@IBOutlet weak var tableView: UITableView!

private var userData = UserData()
private var clearMemoryTimer = Timer.scheduledTimer(timeInterval: 30.0*60.0, target: self, selector: #selector(clearMemory), userInfo: nil, repeats: true)
private let ramCache = NSCache<NSString,UserData>()


let param: Parameters = [
    "order": "desc",
    "max" : 10,
    "sort" : "reputation",
    "site" : "stackoverflow"
]

private var filePath:String{
    let manager = FileManager.default
    let url = manager.urls(for: .documentDirectory, in: .userDomainMask).first
    return url!.appendingPathComponent("SavedData").path
}


override func viewDidLoad() {
print("view loaded")

    super.viewDidLoad()
    self.loadData()

    tableView.delegate = self
    tableView.dataSource = self

    let nibName = UINib(nibName: "CustomTableViewCell", bundle: nil)
    tableView.register(nibName, forCellReuseIdentifier: "tableViewCell")
}


func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
    return self.userData.numberOfUsers()
}

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let cell = tableView.dequeueReusableCell(withIdentifier: "tableViewCell", for: indexPath) as! CustomTableViewCell

    cell.commonInit(image: self.userData.userAtIndex(index: indexPath.item)!.ProfilePicImage!, labelText: self.userData.userAtIndex(index: indexPath.item)!.Username)

    return cell
}

func tableView(_ tableView: UITableView, heightForRowAt indexPath: IndexPath) -> CGFloat {
    return 80
}

func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
    let detailViewController = DetailsUserViewController()
    detailViewController.commonInit(image: self.userData.userAtIndex(index: indexPath.item)!.ProfilePicImage!,
                                    name: self.userData.userAtIndex(index: indexPath.item)!.Username,
                                    location: self.userData.userAtIndex(index: indexPath.item)!.Location,
                                    bronzeBadges: self.userData.userAtIndex(index: indexPath.item)!.BronzeBadges,
                                    silverBadges: self.userData.userAtIndex(index: indexPath.item)!.SilverBadges,
                                    goldBadges: self.userData.userAtIndex(index: indexPath.item)!.GoldBadges)

    self.navigationController?.pushViewController(detailViewController, animated: true)
    self.tableView.deselectRow(at: indexPath, animated: true)
}

private func saveData(user: User) {
    self.userData.appendUser(newUser: user)
    NSKeyedArchiver.archiveRootObject(userData, toFile: filePath)  // we save on disk
    ramCache.setObject(self.userData, forKey: "Data" )
}

private func loadData() {
    if let cachedUsers = self.ramCache.object(forKey: "Data" ) {
        print("data was on ram")
        self.userData.SavedData = cachedUsers.SavedData

    }
    else if let savedUsers = NSKeyedUnarchiver.unarchiveObject(withFile: filePath) as? UserData {
        // check if its on disk
        print("data was on disk")
        self.userData.SavedData = savedUsers.SavedData
        ramCache.setObject(self.userData, forKey: "Data")
    } else {
        print("we requested the data")
        self.requestDataFromApi()
    }
}

func clearMemory()
{
    do{
        try FileManager.default.removeItem(atPath: filePath)
    }
    catch{
        print("Error in clearMemory()")
    }
}



private func requestDataFromApi() {
    // GET the data from the stackexchange api



    Alamofire.request("https://api.stackexchange.com/2.2/users", method: .get, parameters: param).responseJSON { (response) -> (Void) in

        if let json = response.result.value {
            // we got a result

            /* I know this is a bit ugly */
            let json1 = json as! [String:AnyObject]
            let usersInfoFromJSON = json1["items"] as! NSArray       // remember to cast it as NSDictionary


            for userInfo in usersInfoFromJSON {

                let userDict = userInfo as! NSDictionary

                // download user image from url

                Alamofire.request(userDict["profile_image"] as! String).responseData { (response) in
                    if response.error == nil {

                        //print(response.result)

                        if let data = response.data {

                            // save the downloaded image

                            let imageView = UIImageView()
                            imageView.image = UIImage(data: data)

                            // check if user has location set, if not display a proper message
                            var userLocation:String=""

                            if let checkLocation = (userDict["location"] as? String) {
                                userLocation = checkLocation
                            } else {
                                userLocation = "Location is not set."
                            }

                            // get every badge count from the json to use it in User constructor

                            let badgeCounts = userDict["badge_counts"] as? [String:Int]

                            var goldb = 0
                            var bronzeb = 0
                            var silverb = 0

                            if badgeCounts != nil {
                                bronzeb = badgeCounts!["bronze"]!
                                silverb = badgeCounts!["silver"]!
                                goldb   =   badgeCounts!["gold"]!
                            }


                            let newUser = User(username: userDict["display_name"] as! String,
                                               location: userLocation,
                                               bronzeBadges: bronzeb,
                                               silverBadges: silverb,
                                               goldBadges: goldb,
                                               profilePicUrl: userDict["profile_image"] as! String,
                                               profilePicImg: imageView.image)

                            self.saveData(user: newUser)

                            self.tableView.reloadData()

                        }
                    }
                } // end alamofire second request
            } // end user iteration
        }
    } // end alamofire first request
}


 }

Class users:

import Foundation
import UIKit

class User: NSObject,NSCoding {

struct Keys{
    static let Username = "name"
    static let Location = "location"
    static let BronzeBadges = "bronzeb"
    static let SilverBadges = "silverb"
    static let GoldBadges = "goldb"
    static let ProfilePicUrl = "profilePicUrl"
    static let ProfilePicImg = "profilePicImg"
}

var _username:String! = ""
var _location:String! = ""
var _bronzeBadges:Int! = 0
var _silverBadges:Int! = 0
var _goldBadges:Int! = 0
var _profilePicUrl:String! = ""
var _profilePicImage:UIImage?

override init() {}

init(username:String,location:String,bronzeBadges:Int,silverBadges:Int,goldBadges:Int,profilePicUrl:String,profilePicImg:UIImage?) {
    self._username = username
    self._location = location
    self._profilePicUrl = profilePicUrl
    self._profilePicImage = profilePicImg
    self._bronzeBadges = bronzeBadges
    self._silverBadges = silverBadges
    self._goldBadges = goldBadges
}

required init?(coder aDecoder: NSCoder) {
    if let usernameObj = aDecoder.decodeObject(forKey: Keys.Username) as? String {
        _username = usernameObj
    }

    if let locationObj = aDecoder.decodeObject(forKey: Keys.Location) as? String {
        _location = locationObj
    }

    if let bronzeBadgesObj = aDecoder.decodeObject(forKey: Keys.BronzeBadges) as? Int {
        _bronzeBadges = bronzeBadgesObj
    }

    if let silverBadgesObj = aDecoder.decodeObject(forKey: Keys.SilverBadges) as? Int {
        _silverBadges = silverBadgesObj
    }

    if let goldBadgesObj = aDecoder.decodeObject(forKey: Keys.GoldBadges) as? Int {
        _goldBadges = goldBadgesObj
    }

    if let profilePicUrlObj = aDecoder.decodeObject(forKey: Keys.ProfilePicUrl) as? String {
        _profilePicUrl = profilePicUrlObj
    }

    if let profilePicImgObj = aDecoder.decodeObject(forKey: Keys.ProfilePicImg) as? UIImage {
        _profilePicImage = profilePicImgObj
    }
}

func encode(with aCoder: NSCoder) {
    aCoder.encode(_username, forKey: Keys.Username)
    aCoder.encode(_location, forKey: Keys.Location)
    aCoder.encode(_profilePicUrl, forKey: Keys.ProfilePicUrl)
    aCoder.encode(_profilePicImage, forKey: Keys.ProfilePicImg)
    aCoder.encode(_bronzeBadges, forKey: Keys.BronzeBadges)
    aCoder.encode(_silverBadges, forKey: Keys.SilverBadges)
    aCoder.encode(_goldBadges, forKey: Keys.GoldBadges)
}

var Username: String{
    get {
        return _username
    }
    set {
        _username = newValue
    }
}

var Location: String{
    get{
        return _location
    }
    set{
        _location = newValue
    }
}

var BronzeBadges:Int{
    get{
        return _bronzeBadges
    }
    set{
        _bronzeBadges = newValue
    }
}

var SilverBadges:Int{
    get{
        return _silverBadges
    }
    set{
        _silverBadges = newValue
    }
}

var GoldBadges:Int{
    get{
        return _goldBadges
    }
    set{
        _goldBadges = newValue
    }
}

var ProfilePicImage:UIImage?{
    get{
        return _profilePicImage
    }
    set{
        _profilePicImage = newValue
    }
}

var ProfilePicUrl:String{
    get{
        return _profilePicUrl
    }
    set{
        _profilePicUrl = newValue
    }
}
}

Class UserData:

import Foundation

class UserData: NSObject,NSCoding {

    struct Keys{
        static let Data = "data"
    }

    private var _users : [User] = []

    override init() {}

init(with data: [User]) {
    _users = data
}

required init?(coder aDecoder: NSCoder) {
    if let userObj = aDecoder.decodeObject(forKey: Keys.Data) as? [User] {
        _users = userObj
    }

}

func encode(with aCoder: NSCoder) {
    aCoder.encode(_users, forKey: Keys.Data)
}

var SavedData : [User] {
    get {
        return _users
    }
    set {
        _users = newValue
    }
}

func appendUser(newUser: User) {
    _users.append(newUser)
}

func numberOfUsers() -> Int {
    return _users.count
}

func userAtIndex(index: Int) -> User? {
    if(index < _users.count) {
        return _users[index]
    } else {
        print("Error in userAtINdex(() index to big")
        return nil
        }
    }
}

Answer 1

Property accessors

Defining accessor methods for stored properties such as

class User: NSObject, NSCoding {

    var _username:String! = ""

    var Username: String{
        get {
            return _username
        }
        set {
            _username = newValue
        }
    }

    // ...
}

is not necessary in Swift, as documented in Properties in the Swift language reference:

If you have experience with Objective-C, you may know that it provides two ways to store values and references as part of a class instance. In addition to properties, you can use instance variables as a backing store for the values stored in a property.

Swift unifies these concepts into a single property declaration. A Swift property does not have a corresponding instance variable, and the backing store for a property is not accessed directly. This approach avoids confusion about how the value is accessed in different contexts and simplifies the property’s declaration into a single, definitive statement. All information about the property—including its name, type, and memory management characteristics—is defined in a single location as part of the type’s definition.

So in Swift this would simply be

class User: NSObject, NSCoding {

    var userName: String! = ""

    // ...
}

where the property names should be lowerCamelCase according to the API Design Guidelines).

Avoid implicitly unwrapped optionals

As of Swift 3, “Implicitly unwrapped optional” is not a separate type, but an attribute on the declaration of a regular/strong optional, see SE-0054 Abolish ImplicitlyUnwrappedOptional type:

The ImplicitlyUnwrappedOptional ("IUO") type is a valuable tool for importing Objective-C APIs where the nullability of a parameter or return type is unspecified. It also represents a convenient mechanism for working through definite initialization problems in initializers.
...
Except for a few specific scenarios, optionals are always the safer bet, and we’d like to encourage people to use them instead of IUOs.

In particular, implicitly unwrapped optionals are treated as regular (strong) optionals in many cases, which can cause confusion.

So you should declare the user properties as non-optionals or as regular optionals, depending on whether they are required or not. A possible choice would be

class User: NSObject, NSCoding {

    let userName: String
    let location: String?
    let bronzeBadges: Int
    let silverBadges: Int
    let goldBadges: Int
    let profilePicUrl: String?
    let profilePicImage: UIImage?

    // ...
}

Note that I also declared them as constants because the properties are never modified after initialization.

Simplify the decoder, utilize optional chaining

This

    if let locationObj = aDecoder.decodeObject(forKey: Keys.Location) as? String {
        _location = locationObj
    }
    if let bronzeBadgesObj = aDecoder.decodeObject(forKey: Keys.BronzeBadges) as? Int {
        _bronzeBadges = bronzeBadgesObj
    }
    // ...

can now be simplified to

    location = aDecoder.decodeObject(forKey: Keys.location) as? String
    bronzeBadges = aDecoder.decodeObject(forKey: Keys.bronzeBadges) as? Int ?? 0
    // ...

The location property simply becomes nil if decoding fails, and the badges property defaults to zero via optional chaining.

Summarizing the changes so far, the User class looks like this:

class User: NSObject, NSCoding {

    struct Keys{
        static let username = "name"
        static let location = "location"
        static let bronzeBadges = "bronzeb"
        static let silverBadges = "silverb"
        static let goldBadges = "goldb"
        static let profilePicUrl = "profilePicUrl"
        static let profilePicImg = "profilePicImg"
    }

    let userName: String
    let location: String?
    let bronzeBadges: Int
    let silverBadges: Int
    let goldBadges: Int
    let profilePicUrl: String?
    let profilePicImage: UIImage?

    init(userName: String, location: String?, bronzeBadges: Int, silverBadges: Int,
         goldBadges: Int, profilePicUrl: String?, profilePicImage: UIImage?) {
        self.userName = userName
        self.location = location
        self.bronzeBadges = bronzeBadges
        self.silverBadges = silverBadges
        self.goldBadges = goldBadges
        self.profilePicUrl = profilePicUrl
        self.profilePicImage = profilePicImage
    }

    required init?(coder aDecoder: NSCoder) {
        guard let userName = aDecoder.decodeObject(forKey: Keys.username) as? String else {
            return nil
        }
        self.userName = userName

        location = aDecoder.decodeObject(forKey: Keys.location) as? String
        bronzeBadges = aDecoder.decodeObject(forKey: Keys.bronzeBadges) as? Int ?? 0
        silverBadges = aDecoder.decodeObject(forKey: Keys.silverBadges) as? Int ?? 0
        goldBadges = aDecoder.decodeObject(forKey: Keys.goldBadges) as? Int ?? 0
        profilePicUrl = aDecoder.decodeObject(forKey: Keys.profilePicUrl) as? String
        profilePicImage = aDecoder.decodeObject(forKey: Keys.profilePicImg) as? UIImage
    }

    func encode(with aCoder: NSCoder) {
        aCoder.encode(userName, forKey: Keys.username)
        aCoder.encode(location, forKey: Keys.location)
        aCoder.encode(bronzeBadges, forKey: Keys.bronzeBadges)
        aCoder.encode(silverBadges, forKey: Keys.silverBadges)
        aCoder.encode(goldBadges, forKey: Keys.goldBadges)
        aCoder.encode(profilePicUrl, forKey: Keys.profilePicUrl)
        aCoder.encode(profilePicImage, forKey: Keys.profilePicImg)
    }
}

Similar changes can be applied to the UserData class.

Reading the response – Thou shalt not force unwrap!

This refers to the force-unwrap operator ! and the force-cast operator as!, both easily cause fatal errors and unexpected program terminations.

There is a lot of force-casting in the completion handler in the requestDataFromApi() function, starting right at the beginning:

        if let json = response.result.value {
            /* I know this is a bit ugly */
            let json1 = json as! [String:AnyObject]
            let usersInfoFromJSON = json1["items"] as! NSArray
            // ...

That is not ugly, but bad. If for any reason (such as a change in the API)

• the response is not a dictionary, or
• there is no "items" key in the response, or
• the value of the "items" key is not an array

then the force-cast operator as! will cause a runtime exception.

The correct way is to use conditional casts and optional binding:

        if let json = response.result.value as? [String: Any],
            let items = json["items"] as? [[String: Any]] {
            for userDict in items {
                // ...
            }
        }

Note also that the the Foundation types NSArray and NSDictionary are not needed.

Here is a similar problem:

        let badgeCounts = userDict["badge_counts"] as? [String:Int]

        var goldb = 0
        var bronzeb = 0
        var silverb = 0

        if badgeCounts != nil {
            bronzeb = badgeCounts!["bronze"]!
            silverb = badgeCounts!["silver"]!
            goldb   =   badgeCounts!["gold"]!
        }

You check if badgeCounts is not nil, but even then badgeCounts!["bronze"] can be nil and the force-unwrapping causes a crash.

A safe and concise solution is to use optional chaining

        let badgeCounts = userDict["badge_counts"] as? [String:Int]
        let goldb = badgeCounts?["gold"] ?? 0
        let bronzeb = badgeCounts?["silver"] ?? 0
        let silverb = badgeCounts?["bronze"] ?? 0

instead. Now the properties are set to zero if the corresponding entry is not present.