patternswiftMinor
Managing a wish list from an API
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
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
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
Suppose that
The only difference here is the
What this line of code says is basically:
The other important thing here is that we need to be sure that we're making our reference to the delegate a
So, let's make our protocol be an
@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.