Skip to content

Commit

Permalink
Fixing based on mentor suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
Fostahh committed Oct 21, 2022
1 parent 994b280 commit 82dc601
Show file tree
Hide file tree
Showing 11 changed files with 147 additions and 162 deletions.
12 changes: 12 additions & 0 deletions Movie-App-Group-7/Movie-App-Group-7.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
objects = {

/* Begin PBXBuildFile section */
A12FE7232902380800F6166F /* Typealias.swift in Sources */ = {isa = PBXBuildFile; fileRef = A12FE7222902380800F6166F /* Typealias.swift */; };
A14043FC28FFE12F0095BFB6 /* DetailMovieViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = A14043FB28FFE12F0095BFB6 /* DetailMovieViewController.xib */; };
A158A74E2900EF880096F08E /* MovieResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = A158A74D2900EF880096F08E /* MovieResponse.swift */; };
A158A7522900EFC90096F08E /* MovieNetworkModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = A158A7512900EFC90096F08E /* MovieNetworkModel.swift */; };
Expand Down Expand Up @@ -35,6 +36,7 @@
/* End PBXBuildFile section */

/* Begin PBXFileReference section */
A12FE7222902380800F6166F /* Typealias.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Typealias.swift; sourceTree = "<group>"; };
A14043FB28FFE12F0095BFB6 /* DetailMovieViewController.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = DetailMovieViewController.xib; sourceTree = "<group>"; };
A158A74D2900EF880096F08E /* MovieResponse.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MovieResponse.swift; sourceTree = "<group>"; };
A158A7512900EFC90096F08E /* MovieNetworkModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MovieNetworkModel.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -75,6 +77,14 @@
/* End PBXFrameworksBuildPhase section */

/* Begin PBXGroup section */
A12FE721290237E300F6166F /* Typealias */ = {
isa = PBXGroup;
children = (
A12FE7222902380800F6166F /* Typealias.swift */,
);
path = Typealias;
sourceTree = "<group>";
};
A14043FD28FFE4D70095BFB6 /* Extension */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -196,6 +206,7 @@
A15ADAE228FF14B100468D32 /* Utils */ = {
isa = PBXGroup;
children = (
A12FE721290237E300F6166F /* Typealias */,
A14043FD28FFE4D70095BFB6 /* Extension */,
);
path = Utils;
Expand Down Expand Up @@ -399,6 +410,7 @@
BF20612228B1194E00F0915C /* LoginViewController.swift in Sources */,
A15ADADD28FF052300468D32 /* ProfileViewController.swift in Sources */,
BF20611E28B1194E00F0915C /* AppDelegate.swift in Sources */,
A12FE7232902380800F6166F /* Typealias.swift in Sources */,
A158A765290127050096F08E /* UIView+Extension.swift in Sources */,
A15ADAE028FF094400468D32 /* DetailMovieViewController.swift in Sources */,
A158A760290113830096F08E /* ProfileViewModel.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ class UserDefaultModel {
userDefaults = UserDefaults.standard
}

init(userDefaults: UserDefaults) {
self.userDefaults = userDefaults
}

func login(username: String) {
userDefaults.set(username, forKey: usernameUserDefaultsKey)
userDefaults.set(true, forKey: loggedInUserDefaultsKey)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ enum MovieNetworkResult {
}

enum DetailMovieNetworkResult {
case success(Movie?)
case success(Movie)
case failure(String)
}

Expand All @@ -24,64 +24,44 @@ protocol MovieNetworkModel {

final class MovieDefaultNetworkModel : MovieNetworkModel {
func getMovies(completion: @escaping (MovieNetworkResult) -> ()) {
guard let url = URL(string: "https://ghibliapi.herokuapp.com/films") else {
DispatchQueue.main.async {
completion(.failure("Bad URL"))
}
return
}
let url = URL(string: "https://ghibliapi.herokuapp.com/films")!

URLSession.shared.dataTask(with: url) { (data, response, error) in
DispatchQueue.global(qos: .background).async {
DispatchQueue.main.async {
guard let data = data else {
DispatchQueue.main.async {
completion(.success([]))
}
completion(.failure("Data not found"))
return
}

do {
let result = try JSONDecoder().decode([MovieResponse].self, from: data)
let response = ObjectMapper().mapMoviesResponseToMoviesDomain(moviesResponse: result)
DispatchQueue.main.async {
completion(.success(response))
}
completion(.success(response))
} catch {
DispatchQueue.main.async {
completion(.failure("Failed to convert"))
}
completion(.failure("Failed to convert"))
}
}


}.resume()
}

func getMovie(movieId: String, completion: @escaping (DetailMovieNetworkResult) -> ()) {
guard let url = URL(string: "https://ghibliapi.herokuapp.com/films/\(movieId)") else {
DispatchQueue.main.async {
completion(.failure("Bad URL"))
}
return
}
let url = URL(string: "https://ghibliapi.herokuapp.com/films/\(movieId)")!

URLSession.shared.dataTask(with: url) { (data, response, error) in
DispatchQueue.global(qos: .background).async {
DispatchQueue.main.async {
guard let data = data else {
DispatchQueue.main.async {
completion(.success(nil))
}
completion(.failure("Data not found"))
return
}

do {
let result = try JSONDecoder().decode(MovieResponse.self, from: data)
let response = ObjectMapper().mapMovieResponseToMovieDomain(movieResponse: result)
DispatchQueue.main.async {
completion(.success(response))
}
completion(.success(response))
} catch {
DispatchQueue.main.async {
completion(.failure("Failed to convert"))
}
completion(.failure("Failed to convert"))
}
}
}.resume()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ class DetailMovieViewController: UIViewController {
@IBOutlet private weak var movieDirectorLabel: UILabel!
@IBOutlet private weak var movieDescriptionLabel: UILabel!
@IBOutlet private weak var scrollView: UIScrollView!
@IBOutlet private weak var errorLabel: UILabel!
@IBOutlet private weak var contentView: UIView!

private let viewModel = DetailMovieViewModel(movieNetworkModel: MovieDefaultNetworkModel())

Expand All @@ -29,33 +31,34 @@ class DetailMovieViewController: UIViewController {
self.tabBarController?.tabBar.isHidden = true

guard let movieId = movieId else {
self.navigationController?.popViewController(animated: true)
self.showError(message: "Movie ID not found", true)
return
}

bindObservers()
viewModel.retrieveMovie(movieId: movieId)
configureScrollView()
retrieveDetailMovie(movieId)
}

override func viewWillDisappear(_ animated: Bool) {
super.viewWillDisappear(animated)
self.tabBarController?.tabBar.isHidden = false
}

private func retrieveDetailMovie(_ movieId: String) {
private func bindObservers() {
viewModel.movieObservable = { [weak self] movie in
self?.bindView(movie)
}

viewModel.retrieveMovie(movieId: movieId) { [weak self] result in
switch result {
case .success(let optionalMovie):
guard let movie = optionalMovie else {
self?.navigationController?.popViewController(animated: true)
return
}
self?.bindView(movie)
case .failure(let message):
print(message)
viewModel.showErrorMessage = { [weak self] message in
if let message = message {
self?.showError(message: message, true)
}
}

viewModel.showLoading = { [weak self] isLoading in
self?.contentView.isShimmering = isLoading
}
}

private func configureScrollView() {
Expand All @@ -77,48 +80,32 @@ class DetailMovieViewController: UIViewController {
}

private func loadImageUsingKingfisher(url: URL, urlPoster: URL) {
isDownloadingImage(true, movieImageContainerView)
isDownloadingImage(true, movieBannerContainerView)

movieImageView.kf.setImage(
with: url,
placeholder: nil,
options: nil,
progressBlock: { [weak self] _, _ in
self?.isDownloadingImage(true, self?.movieImageContainerView)
},
completionHandler: { [weak self] result in
switch result {
case .success(_):
self?.isDownloadingImage(false, self?.movieImageContainerView)
case .failure(let error):
self?.isDownloadingImage(false, self?.movieImageContainerView)
print(error)
}
}
)
downloadingImage(for: movieImageView, with: url, movieImageContainerView)
downloadingImage(for: movieBannerImageView, with: urlPoster, movieBannerContainerView)
}

private func downloadingImage(for imageView: UIImageView, with url: URL, _ view: UIView?) {
isDownloadingImage(true, view)

movieBannerImageView.kf.setImage(
with: urlPoster,
placeholder: nil,
options: nil,
progressBlock: { [weak self] _, _ in
self?.isDownloadingImage(true, self?.movieBannerContainerView)
},
completionHandler: { [weak self] result in
switch result {
case .success(_):
self?.isDownloadingImage(false, self?.movieBannerContainerView)
case .failure(let error):
self?.isDownloadingImage(false, self?.movieBannerContainerView)
print(error)
}
imageView.kf.setImage(with: url) { [weak self] result in
switch result {
case .success(_):
self?.isDownloadingImage(false, view)
case .failure(let error):
self?.isDownloadingImage(false, view)
print(error)
}
)
}
}

private func isDownloadingImage(_ isDownloading: Bool, _ view: UIView?) {
view?.isShimmering = isDownloading
view?.backgroundColor = isDownloading ? .gray : .clear
}

private func showError(message: String? = nil, _ isError: Bool = false) {
errorLabel.isHidden = !isError
scrollView.isHidden = isError
errorLabel.text = message
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
<objects>
<placeholder placeholderIdentifier="IBFilesOwner" id="-1" userLabel="File's Owner" customClass="DetailMovieViewController" customModule="Movie_App_Group_7" customModuleProvider="target">
<connections>
<outlet property="contentView" destination="RxM-X9-5CQ" id="4OP-CZ-IgS"/>
<outlet property="errorLabel" destination="6ch-zQ-FxC" id="MhU-gm-kr7"/>
<outlet property="movieBannerContainerView" destination="yeF-4K-tfk" id="j76-aX-Pls"/>
<outlet property="movieBannerImageView" destination="Fet-7o-VvU" id="TuQ-8L-wjY"/>
<outlet property="movieDescriptionLabel" destination="Lv0-Bh-hmm" id="ky0-Iu-vWU"/>
Expand All @@ -29,11 +31,17 @@
<rect key="frame" x="0.0" y="0.0" width="414" height="896"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMaxY="YES"/>
<subviews>
<label hidden="YES" opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251" text="" textAlignment="natural" lineBreakMode="tailTruncation" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="6ch-zQ-FxC" userLabel="errorLabel">
<rect key="frame" x="207" y="448" width="0.0" height="0.0"/>
<fontDescription key="fontDescription" type="system" pointSize="25"/>
<color key="textColor" systemColor="systemRedColor"/>
<nil key="highlightedColor"/>
</label>
<scrollView clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="PEu-Jr-vm8">
<rect key="frame" x="0.0" y="0.0" width="414" height="862"/>
<subviews>
<view contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="RxM-X9-5CQ">
<rect key="frame" x="0.0" y="0.0" width="414" height="518.5"/>
<rect key="frame" x="0.0" y="0.0" width="414" height="506.5"/>
<subviews>
<view contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="434-lg-BDT" userLabel="MovieImage ContainerView">
<rect key="frame" x="0.0" y="0.0" width="414" height="250"/>
Expand All @@ -52,7 +60,7 @@
</constraints>
</view>
<view contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="yeF-4K-tfk">
<rect key="frame" x="16" y="266" width="125" height="200"/>
<rect key="frame" x="16" y="254" width="125" height="200"/>
<subviews>
<imageView clipsSubviews="YES" userInteractionEnabled="NO" contentMode="scaleToFill" horizontalHuggingPriority="251" verticalHuggingPriority="251" translatesAutoresizingMaskIntoConstraints="NO" id="Fet-7o-VvU">
<rect key="frame" x="0.0" y="0.0" width="125" height="200"/>
Expand All @@ -69,7 +77,7 @@
</constraints>
</view>
<stackView opaque="NO" contentMode="scaleToFill" axis="vertical" spacing="8" translatesAutoresizingMaskIntoConstraints="NO" id="LsC-r4-jXP">
<rect key="frame" x="157" y="266" width="241" height="200"/>
<rect key="frame" x="157" y="254" width="241" height="200"/>
<subviews>
<label opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="252" verticalCompressionResistancePriority="749" text="Title" textAlignment="natural" lineBreakMode="tailTruncation" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="CyW-Kr-pqL">
<rect key="frame" x="0.0" y="0.0" width="241" height="20.5"/>
Expand Down Expand Up @@ -104,7 +112,7 @@
</subviews>
</stackView>
<label opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251" usesAttributedText="YES" lineBreakMode="tailTruncation" numberOfLines="0" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="Lv0-Bh-hmm">
<rect key="frame" x="16" y="482" width="382" height="20.5"/>
<rect key="frame" x="16" y="470" width="382" height="20.5"/>
<attributedString key="attributedText">
<fragment content="Description">
<attributes>
Expand All @@ -122,7 +130,7 @@
<constraint firstAttribute="trailing" secondItem="LsC-r4-jXP" secondAttribute="trailing" constant="16" id="1GH-1R-4s9"/>
<constraint firstItem="LsC-r4-jXP" firstAttribute="bottom" secondItem="yeF-4K-tfk" secondAttribute="bottom" id="22J-HZ-gr4"/>
<constraint firstAttribute="trailing" secondItem="434-lg-BDT" secondAttribute="trailing" id="Coq-ts-rNq"/>
<constraint firstItem="yeF-4K-tfk" firstAttribute="top" secondItem="434-lg-BDT" secondAttribute="bottom" constant="16" id="DBd-VT-9vC"/>
<constraint firstItem="yeF-4K-tfk" firstAttribute="top" secondItem="434-lg-BDT" secondAttribute="bottom" constant="4" id="DBd-VT-9vC"/>
<constraint firstItem="Lv0-Bh-hmm" firstAttribute="leading" secondItem="RxM-X9-5CQ" secondAttribute="leading" constant="16" id="GY3-dE-quP"/>
<constraint firstAttribute="bottom" secondItem="Lv0-Bh-hmm" secondAttribute="bottom" constant="16" id="JAz-QK-rqn"/>
<constraint firstAttribute="trailing" secondItem="Lv0-Bh-hmm" secondAttribute="trailing" constant="16" id="L7a-fy-EWq"/>
Expand Down Expand Up @@ -152,6 +160,8 @@
<constraint firstItem="RxM-X9-5CQ" firstAttribute="width" secondItem="mMg-u2-bVM" secondAttribute="width" id="Xap-16-RpO"/>
<constraint firstItem="7J1-c1-USN" firstAttribute="trailing" secondItem="PEu-Jr-vm8" secondAttribute="trailing" id="fGr-e4-7jN"/>
<constraint firstItem="7J1-c1-USN" firstAttribute="bottom" secondItem="PEu-Jr-vm8" secondAttribute="bottom" id="goY-Pf-UIS"/>
<constraint firstItem="6ch-zQ-FxC" firstAttribute="centerY" secondItem="mMg-u2-bVM" secondAttribute="centerY" id="mfU-YQ-wVq"/>
<constraint firstItem="6ch-zQ-FxC" firstAttribute="centerX" secondItem="mMg-u2-bVM" secondAttribute="centerX" id="xQj-VV-xFR"/>
</constraints>
<point key="canvasLocation" x="139" y="97"/>
</view>
Expand All @@ -160,5 +170,8 @@
<systemColor name="systemBackgroundColor">
<color white="1" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/>
</systemColor>
<systemColor name="systemRedColor">
<color red="1" green="0.23137254901960785" blue="0.18823529411764706" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
</systemColor>
</resources>
</document>
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,29 @@

import Foundation

enum RetrievingDetailMovieState {
case success(Movie?)
case failure(String)
}

class DetailMovieViewModel {
var showLoading: Observer<Bool>?
var showErrorMessage: Observer<String?>?
var movieObservable: Observer<Movie>?

private let movieNetworkModel : MovieNetworkModel

init(movieNetworkModel: MovieNetworkModel) {
self.movieNetworkModel = movieNetworkModel
}

func retrieveMovie(movieId: String, completion : @escaping (RetrievingDetailMovieState) -> ()) {
movieNetworkModel.getMovie(movieId: movieId) { result in
func retrieveMovie(movieId: String) {
showLoading?(true)
showErrorMessage?(nil)

movieNetworkModel.getMovie(movieId: movieId) { [weak self] result in
switch result {
case .success(let movie):
completion(.success(movie))
self?.showLoading?(false)
self?.movieObservable?(movie)
case .failure(let message):
completion(.failure(message))
self?.showLoading?(false)
self?.showErrorMessage?(message)
}
}

Expand Down
Loading

0 comments on commit 82dc601

Please sign in to comment.