Untitled Pull Request #47
Conversation
|
|
||
|
|
||
|
|
||
| class GenericMovieCollectionView: UICollectionView { |
There was a problem hiding this comment.
Please avoid adding commented code to your commits.
|
|
||
| } | ||
|
|
||
| extension GenericTableViewController: UITableViewDelegate, UITableViewDataSource { |
There was a problem hiding this comment.
Same here: Please avoid adding commented code to your commits.
|
|
||
| class HomeViewController: UIViewController, HomeView { | ||
|
|
||
| var arrMovies: [SectionMovie: [Movie]]! |
There was a problem hiding this comment.
Please avoid force unwrap properties "!"
| class HomeViewController: UIViewController, HomeView { | ||
|
|
||
| var arrMovies: [SectionMovie: [Movie]]! | ||
| var dataSource: UICollectionViewDiffableDataSource<SectionMovie, Movie>! = nil |
There was a problem hiding this comment.
Same here: Please avoid force unwrap properties "!"
| return searchController | ||
| }() | ||
|
|
||
| var movieHomeCollectionView: UICollectionView! |
There was a problem hiding this comment.
Same here: Please avoid force unwrap properties "!"
You can declare this view like this: private lazy var movieHomeCollectionView: UICollectionView = { }()
The same you did with the searchController.
| var tableView: UITableView! | ||
|
|
||
| func showEmptyState() { | ||
| print("") |
There was a problem hiding this comment.
If this is meant to be updated in the future please try to add // TODO: comments in the method's body.
| let voteCount: Int? | ||
| let popularity: Double? | ||
|
|
||
| enum CodingKeys: String, CodingKey { |
There was a problem hiding this comment.
Do you think this CodingKeys declaration can be private?
TheMovieDb/Models/Movie.swift
Outdated
| case popularity | ||
| } | ||
|
|
||
| func hash(into hasher: inout Hasher) { |
There was a problem hiding this comment.
Just a question: Why do you require this struct to be Hashable?
| decode: @escaping (Decodable) -> T?, | ||
| completion: @escaping (Result<T, APIError>) -> Void) { | ||
| let task = decodingTask(with: request, decodingType: T.self) { (json, error) in | ||
| DispatchQueue.main.async { |
There was a problem hiding this comment.
Just a question: Should the APIClient be responsible for handling the response in the main thread?
|
|
||
| var title: String? { | ||
| didSet { | ||
| configure() |
There was a problem hiding this comment.
I have my doubts about this configure() method each time you assign these properties.
Interface Home MVP