The oauthgistgithub from vwolfg

Code Review

A crash on this line:

OAuthGistGitHub/GistApp/GistsList/View/GistViewController.swift

Line 74 in ec1dbce

self.token = keychain.string(forKey: "access_token") as! String

The following code I'd rewrite:

OAuthGistGitHub/GistApp/GistsList/Service/GistsViewModel.swift

Lines 14 to 37 in ec1dbce

    
           class GistsViewModel { 
        
               private var gists = [Gist]() 
        
               private let keychain = KeychainManager.sharedKeychainWrapper 
        
               func loadGists(completion: @escaping ([Gist]) -> Void){ 
        
                   guard let url = URL(string: Constans.url) else {return} 
        
                   let headers = ["Authorization":"token \(keychain.string(forKey: "access_token") ?? "")"] 
        
                   Alamofire.request(url, method: .get, parameters: nil, encoding: JSONEncoding.default, headers: headers).validate().responseData { (data) in 
        
                       switch data.result { 
        
                       case .success(let value): 
        
                           do { 
        
                               self.gists = try JSONDecoder().decode([Gist].self, from: value) 
        
                               completion(self.gists) 
        
                           } catch let error { 
        
                               print("Gists uploading error \(error)") 
        
                           } 
        
                       case .failure(let err): 
        
                           print("Gists uploading error \(err)") 
        
                       } 
        
                   } 
        
               } 
        
           }

class GistsViewModel {
    private let keychain = KeychainManager.sharedKeychainWrapper
    func loadGists(completion: @escaping ([Gist]) -> Void){
        let url = URL(string: Constans.url)!
        let headers = ["Authorization":"token \(keychain.string(forKey: "access_token") ?? "")"]
        Alamofire.request(url, method: .get, parameters: nil, encoding: JSONEncoding.default, headers: headers).validate().responseData { (data) in
            switch data.result.flatMap({ try JSONDecoder().decode([Gist].self, from: $0) }) {
            case .success(let gists):
                completion(gists)
            case .failure(let err):
                completion([])
                print("Gists uploading error \(err)")
            }
        }
    }
}

Note:

there is no reason to keep gists in the model (at least you don't use it anywhere)
you can guarantee that the url is correct, hence force unwrap it
usage of flatMap with decoding for a result
completion is called in both success and failure branches. What if a user of this code wants to show a spinner and hide it on completion?

Other improvements could be: propagate errors to the caller, for example using Result<[Gist]> in completion

Seems like you don't need isFirstLoading here:

OAuthGistGitHub/GistApp/GistsList/View/GistViewController.swift

Lines 28 to 30 in ec1dbce

    
           super.viewDidLoad() 
        
           if isFirstLoading { 
        
               uploadData()

vwolfg / oauthgistgithub Goto Github PK

oauthgistgithub's People

Contributors

Watchers

oauthgistgithub's Issues

Code Review

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	class GistsViewModel {

	private var gists = [Gist]()
	private let keychain = KeychainManager.sharedKeychainWrapper
	func loadGists(completion: @escaping ([Gist]) -> Void){
	guard let url = URL(string: Constans.url) else {return}
	let headers = ["Authorization":"token \(keychain.string(forKey: "access_token") ?? "")"]
	Alamofire.request(url, method: .get, parameters: nil, encoding: JSONEncoding.default, headers: headers).validate().responseData { (data) in
	switch data.result {
	case .success(let value):
	do {
	self.gists = try JSONDecoder().decode([Gist].self, from: value)
	completion(self.gists)
	} catch let error {
	print("Gists uploading error \(error)")
	}
	case .failure(let err):
	print("Gists uploading error \(err)")
	}
	}
	}


	}