cklynch2
commented
8 years ago Dylan,
On behalf of Team Wolf, thank you!! We are incredibly grateful for your feedback and will work on implementing these changes. More than anything, we want to express our gratitude for the fact that you took the time not only to suggest improvements, but also to explain why these changes are a better approach. Considering how early we are in the process of learning to code, it is invaluable to understand how AND why.
It would be awesome to receive regular input. Please let us know if email is your preferred way of communicating; we can adjust based on whatever is most convenient for you.
Thanks again, and have an awesome Thursday! Cheers, Flatiron Wolves
On Thu, Aug 11, 2016 at 2:47 AM, Dylan Shine notifications@github.com wrote:
@cklynch2 https://github.com/cklynch2 @bmanjuu https://github.com/bmanjuu @zainnadeem https://github.com/zainnadeem @saraahmad1114 https://github.com/saraahmad1114
Hey Team Wolf! I see that you've been making some good progress over the past week or so, and I just wanted to start to chime in with some tips, tricks & feedback on your project. Despite the negative connotation Github Issues may have, I'm simply going to use it as a way to communicate directly with you guys on the repo. With that being said, lets get started:
For this bit of feedback I wanted to go over the structuring of your API services/clients. So the basic objective is to make a network request to get some data, and then transform that data into something our application can use, such as a Location.
Where should this request take place? Some people put this sort of logic right into their view controllers, which leaves them with the Massive View Controller (MVC...get it...bad joke) problem that we're trying to avoid. Team Wolf instinctively did the right thing by separating that logic out into their own classes which is 100% the right move. In a traditional MVC pattern, I can justify having the view controller initiate another object to make a request, but not itself.
What type should this client object be? In Swift, the types we choose for our objects is very important. For each client, you decided to go with a class. At the end of the day, a class will do the trick, but lets think about what we are trying to accomplish. As we all know, the main feature classes provide us with is inheritance, which is a great tool for structuring object hierarchies. I see your client as a stateless interface used to interact with an particular API. Since this is the case, I would suggest utilizing a struct. Swift favors value types over reference types, for a whole slew of reasons which I don't need to get into, but the main take away is that we should carefully choose our types based on we plan to accomplish. Since our clients don't need to leverage inheritance, then lets opt for a struct. This would be no different from choosing an immutable array over a mutable one...if we don't need to mutate the collection, then why should we allow it to do so.
How should I handle the response? Refer to example code below: So we've successfully made a request, and now we want to handle the response. In the completion, I first like to employ the Fail First/Fast approach. This approach is simply checking if our request failed, and if so, invoking our completion by passing through the error. The Fail First/Fast allows the minimum amount work from executing given a bad request. Secondly we should be passing our error, especially if a view controller is the one kicking off the request so we can properly update our view for error states.
If the request is successful, and we have valid JSON, we pass the JSON to another object to create our models, and pass the models in the completion. If we are given invalid JSON, we simply pass through an error in the completion.
struct APIService {
private struct Constants { static let baseURL = "http://api.com" static let key = "secret" }
enum RequestURLs: String { case Models = "/models"
var url: String { return Constants.baseURL + rawValue }}
enum APIError: ErrorType { case InvalidJSON }
typealias ModelCompletion = ([Model], ErrorType?) -> () typealias JSONArray = [[String: AnyObject]]
static func getModels(completion: ModelCompletion) { Alamofire.request(.GET, RequestURLs.Models.url, parameters: ["key": Constants.key]) .responseJSON { response in
switch response.result { case .Failure(let error): completion([Model](), error) case .Success(let value): if let json = value as? JSONArray { completion(Model.modelsWithJSON(json), nil) } else { completion([Model](), APIError.InvalidJSON) } } }}
struct Model { let id: String
static func modelsWithJSON(json: [[String: AnyObject]]) -> [Model] { return json.flatMap(self.init) }
init?(json: [String: AnyObject]) { guard let id = json["id"] as? String else { return nil } self.id = id }
}
Of course this is a super contrived example. The points I wanted to make were giving our request initiators the option to respond to errors, and delegating object creation responsibilities to another object.
Alright, that's going to do it for this feedback. I just want to iterate that writing software is as much of an art as it is a science. This is a style that I've used and works for me. Take the time and explore how other developers & projects implement service classes.
If you found this helpful and would like me to write more feedback like this or have any questions , please let me know.
Keep on hacking!
Best, Dylan Shine
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/learn-co-students/ios-0616-team-wolf/issues/2, or mute the thread https://github.com/notifications/unsubscribe-auth/APZJHGxgwkvoksARi-qphqgK5Cc53GBoks5qesWLgaJpZM4Jh1si .
@cklynch2 @bmanjuu @zainnadeem @saraahmad1114
Hey Team Wolf! I see that you've been making some good progress over the past week or so, and I just wanted to start to chime in with some tips, tricks & feedback on your project. Despite the negative connotation Github Issues may have, I'm simply going to use it as a way to communicate directly with you guys on the repo. With that being said, lets get started:
For this bit of feedback I wanted to go over the structuring of your API services/clients. So the basic objective is to make a network request to get some data, and then transform that data into something our application can use, such as a Location.
Where should this request take place? Some people put this sort of logic right into their view controllers, which leaves them with the Massive View Controller (MVC...get it...bad joke) problem that we're trying to avoid. Team Wolf instinctively did the right thing by separating that logic out into their own classes which is 100% the right move. In a traditional MVC pattern, I can justify having the view controller initiate another object to make a request, but not implement networking logic itself.
What type should this client object be? In Swift, the types we choose for our objects are very important. For each client, you decided to go with a class. At the end of the day, a class will do the trick, but lets think about what we are trying to accomplish. As we all know, the main feature classes provide us with is inheritance, which is a great tool for structuring object hierarchies. I see your client as a stateless interface used to interact with a particular API. Since this is the case, I would suggest utilizing a struct. Swift favors value types over reference types, for a whole slew of reasons which I don't need to get into, but the main take away is that we should carefully choose our types based on we plan to accomplish. Since our clients don't need to leverage inheritance, then lets opt for a struct. This would be no different from choosing an immutable array over a mutable one...if we don't need to mutate the collection, then why should we allow it to do so.
How should I handle the response? Refer to example code below: So we've successfully made a request, and now we want to handle the response. In the completion, I first like to employ the Fail First/Fast approach. This approach is simply checking if our request failed, and if so, invoking our completion by passing through the error. The Fail First/Fast allows the minimum amount work from executing given a bad request. Secondly we should be passing our error, especially if a view controller is the one kicking off the request so we can properly update our view for error states.
If the request is successful, and we have valid JSON, we pass the JSON to another object to create our models, and pass the models in the completion. If we are given invalid JSON, we simply pass through an error in the completion.
Of course this is a super contrived example. The points I wanted to make were giving our request initiators the option to respond to errors, and delegating object creation responsibilities to another object. Alright, that's going to do it for this feedback. I just want to iterate that writing software is as much of an art as it is a science. This is a style that I've used and works for me. Take the time and explore how other developers & projects implement service classes.
If you found this helpful and would like me to write more feedback like this or have any questions, please let me know.
Keep on hacking!
Best, Dylan Shine