Finished development. #1
Conversation
Also made a little change on the UITests.
| import RxSwift | ||
| import RxCocoa | ||
|
|
||
| class BaseViewController<T>: UIViewController { |
There was a problem hiding this comment.
Why did you decide to create a BaseViewController instead of a protocol?
| import Foundation | ||
| import ObjectMapper | ||
|
|
||
| class Movie: Mappable { |
There was a problem hiding this comment.
How would you do to avoid a 3rd party to do this same mapping?
There was a problem hiding this comment.
I would use the Codale protocol, which includes encoding and decoding. The reason I choose to use ObjectMapper is for being able to work with dictionaries, instead of Data objects.
| } | ||
|
|
||
| func upcomingMovies(for page: Int) -> Observable<[Movie]> { | ||
| if page <= lastUpcomingMoviesPage ?? 100 { |
There was a problem hiding this comment.
Where this 100 comes from? What it means?
| availableGenres().map { $0 as AnyObject }) | ||
| .merge() | ||
| .asObservable() | ||
| .toArray().map({ (result) -> [Movie] in |
There was a problem hiding this comment.
Could you tell me the advantages and disadvantages of using .toArray()?
There was a problem hiding this comment.
toArray() basically gets a sequence observable, which is an observable for each item and turns them into one observable, making it possible to make n requests and return a single response.
I'm not aware of any disadvantages in using it.
| return Mapper<Genre>().mapArray(JSONArray: genresJson) | ||
| } | ||
| } catch { | ||
|
|
There was a problem hiding this comment.
How would you handle the exception caught here?
| self.movie.accept(detailedMovie) | ||
|
|
||
| case .error(let error): | ||
| Log.error("Error: \(error.localizedDescription)") |
There was a problem hiding this comment.
In case of an error, you're only login it for development purpose. How would you handle it so the user can tell that an error has occurred?
There was a problem hiding this comment.
I would create another behaviourRelay, an BehaviourRelay<Error?>, and would subscribe to it on any needed view. Whenever I got an error in that log, go through error.accept(error) and let the view show to the user the way it can. In that case, I would show an alert to the user, so he can try again in a few minutes.
| if let releaseDate = movie.releaseDate { | ||
| self.lblReleaseYear.text = releaseDate.string(with: .medium) | ||
| } | ||
| }).disposed(by: disposeBag) |
There was a problem hiding this comment.
When this subscription is going to be disposed?
There was a problem hiding this comment.
As soon as it's owner is released, in that case, the MovieDetailViewController.
| .orEmpty | ||
| .debounce(0.5, scheduler: MainScheduler.instance) | ||
| .asDriver(onErrorJustReturn: "") | ||
| .drive((viewModel?.searchText)!) |
There was a problem hiding this comment.
I noticed that you use force unwrap quite
Is this a pattern that you always follow?
If not, how would you handle a crash in this situation?
There was a problem hiding this comment.
I only do force unwrap in situations I'm sure it won't ever be nil, in the commented part, for instance, I'm always sure that viewModel and searchText will always be instantiated when this method is called.
Whenever I can't be sure (dealing with objects that come from a server, for instance) I use if let or guard let, to make sure.
No description provided.