-
Notifications
You must be signed in to change notification settings - Fork 0
MVP #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MVP #2
Conversation
|
|
||
| final class APIService: APIServiceProtocol { | ||
|
|
||
| private let baseURL: String = "https://d3jbb8n5wk0qxi.cloudfront.net/recipes.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future Upgrade
It would be nice to be able to initialize the API with whatever endpoint we want. Might want to find a mechanism in the next version to do this.
| print("Error with HTTP response: \(String(describing: response))") | ||
| return [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future Upgrade
Better error handling here would be nice and would allow for a better UX. Also would probably be nice to see a difference between malformed data and empty data.
| private let baseURL: String = "https://d3jbb8n5wk0qxi.cloudfront.net/recipes.json" | ||
|
|
||
| func fetchRecipies() async -> [Recipe] { | ||
| let url = URL(string: baseURL)! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stability Improvement
Probably should be using a guard statement here when we allow for any URL to be passed into the client.
|
|
||
| init(apiService: APIServiceProtocol = APIService()) { | ||
| self.apiService = apiService | ||
| Task { await self.fetchRecipies()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug Exploration
This might not be thread-safe? Should look into this more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stability Improvement
Probably would be better to break this out into some components. I feel like there is some redundant code but also just some clear reusable components that could be made here.
What is this?
This pull request is for the MVP version of the Fetch iOS Software Engineer take-home assessment. It features the ability to fetch recipes from an API, refresh the list of recipes, and deal with empty and malformed data.
What's included?
This PR includes:
RecipemodelAPIServiceRecipeViewModelRecipeViewAsyncImageis used for caching and loading images in phases.Overall Testing
Testing Data Retrieval From Standard API
Testing for data retrieval from the standard recipes API is straightforward, and it is the currently loaded endpoint is inside of the
APIClient.Testing Data Retrieval From Malformed Data API
In order to test for this, replace line 12 in
APIClientwithTesting Data Retrieval From Empty Data API
In order to test for this, replace line 12 in
APIClientwithTesting Images / Videos
Standard Data API
Malformed & Empty Data APIs