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

Managing a wish list from an API

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
managingwishlistfromapi

Problem

I have a class that I use to manage a list to/from an API in Swift. Using a clean code approach, I'm wondering if, how, and why I should change it when I have a class that uses it for just one or two functions.

Here's the class:

```
import Foundation

protocol ApiWishListManagerDelegate
{
func didFailWishList(failError: NSError?)
func didGetWishList()
func didAddToWishList()
func didUpdate()
func didDelete()
}

class ApiMyAccountWishListManager: ApiController
{
// MARK: - Vars
var delegateWishList: ApiWishListManagerDelegate?

// MARK: - Inits

override init()
{
super.init()
}

init(delegateWishList: ApiWishListManagerDelegate)
{
super.init()
self.delegateWishList = delegateWishList
}

// MARK: - Functions

func addToWishList(productId: Int)
{
var dataToPost = NSDictionary(objects: [productId], forKeys: ["pid"])
post(MakeURL.addToWishList(), postData: dataToPost, callBack: parseAddToWishList)
}

func parseAddToWishList(data: NSData?, response: NSURLResponse, error: NSError?)
{
if let jsonResultDictionary = convertOnSuccessAPI(response, data: data!)
{
self.delegateWishList!.didAddToWishList()
return
}
self.delegateWishList!.didFailWishList(error)
}

func readItems()
{
get(MakeURL.getWishList(), queryString: "", https: true, callBack: parseReadItems)
}

func parseReadItems(data: NSData?, response: NSURLResponse, error: NSError?)
{
if let jsonResultDictionary = convertOnSuccessAPI(response, data: data!)
{
MyAccountDataCache.sharedInstance.saveWishList(jsonResultDictionary)
self.delegateWishList!.didGetWishList()
return
}
self.delegateWishList!.didFailWishList(error)
}

func updateItemQuantity(productId: Int, quantity: Int, folderId: Int)
{
var dataToPost = NSDictionary(objec

Solution

We should definitely go ahead and use @objc and make the methods optional that make sense to be optional.

In Swift, it's actually not nearly as bad to check whether or not a delegate is set up or responds to a selector. In Objective-C, it was a big if statement. In Swift, we can use optional chaining.

Suppose that didUpdate is an optional method. Calling it on our delegate then looks like this:

self.delegate?.didUpdate?()


The only difference here is the ? before the parenthesis.

What this line of code says is basically:

if delegate is not nil and if delegate responds to selector "didUpdate"
call "didUpdate" on delegate
else
do nothing


The other important thing here is that we need to be sure that we're making our reference to the delegate a weak reference, which can only be done if our protocol is specified as @objc. In almost every case, the object acting as our delegate is already holding a strong reference to the class that needs the delegate. If this class also holds a strong reference back to the delegate (which you're currently doing), we've creating a retain cycle. The memory can never be released without annoying setting each other's reference to the other to be nil... and the hard part is often times figuring out when to do that.

So, let's make our protocol be an @objc protocol and use a weak reference to our delegate, and then use optional chaining to take care of the optional methods.

Code Snippets

self.delegate?.didUpdate?()

Context

StackExchange Code Review Q#93357, answer score: 3

Revisions (0)

No revisions yet.