Ghibli Movie APp #4

Fostahh · 2022-10-20T07:51:16Z

No description provided.

zmkhtr

Azri.

zmkhtr · 2022-10-20T13:39:39Z

...e-App-Group-7/Movie-App-Group-7/Core/Data/Remote Data Source/Network/MovieNetworkModel.swift

+        guard let url = URL(string: "https://ghibliapi.herokuapp.com/films") else {
+            DispatchQueue.main.async {
+                completion(.failure("Bad URL"))
+            }
+            return
+        }


Jangan pake guard let buat unwrap URL disini, kenapa? karena klo salah disini itu developer mistake bukan codenya, jadi untuk mencegah hal tersebut lebih baik ini di force unwrap.

Suggested change

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")!

Mau nanya Bang. Semisalnya invalid URL, ini ga ngecrash Bang kalo di force unwrap?

zmkhtr · 2022-10-20T13:40:06Z

...e-App-Group-7/Movie-App-Group-7/Core/Data/Remote Data Source/Network/MovieNetworkModel.swift

+        guard let url = URL(string: "https://ghibliapi.herokuapp.com/films/\(movieId)") else {
+            DispatchQueue.main.async {
+                completion(.failure("Bad URL"))
+            }
+            return
+        }


Sama ini juga kyk yg diatas

Suggested change

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)")!

zmkhtr · 2022-10-20T13:41:17Z

...e-App-Group-7/Movie-App-Group-7/Core/Data/Remote Data Source/Network/MovieNetworkModel.swift

+            DispatchQueue.global(qos: .background).async {
+                guard let data = data else {
+                    DispatchQueue.main.async {
+                        completion(.success(nil))


Throw error disini jangan success kalau emang datanya gaada, completion aja dengan completion(.failure("Data not found"))

zmkhtr · 2022-10-20T13:42:55Z

...e-App-Group-7/Movie-App-Group-7/Core/Data/Remote Data Source/Network/MovieNetworkModel.swift

+        URLSession.shared.dataTask(with: url) { (data, response, error) in
+            DispatchQueue.global(qos: .background).async {
+                guard let data = data else {
+                    DispatchQueue.main.async {
+                        completion(.success(nil))
+                    }
+                    return
+                }
+
+                do {
+                    let result = try JSONDecoder().decode(MovieResponse.self, from: data)
+                    let response = ObjectMapper().mapMovieResponseToMovieDomain(movieResponse: result)
+                    DispatchQueue.main.async {
+                        completion(.success(response))
+                    }
+                } catch {
+                    DispatchQueue.main.async {
+                        completion(.failure("Failed to convert"))
+                    }
+                }
+            }
+        }.resume()


dari pada manggil DispatchQueue.main.async banyak'' bisa lu ganti aja tuh yg di line 67 DispatchQueue.global(qos: .background).async jadi langsung pake yg DispatchQueue.main.async. Karena URLSession defaultnya udah di background thread

Mapping object di Main thread gamasalah Bang?

zmkhtr · 2022-10-20T13:44:38Z

Movie-App-Group-7/Movie-App-Group-7/UI/Detail Movie/DetailMovieViewController.swift

+        guard let movieId = movieId else {
+            self.navigationController?.popViewController(animated: true)
+            return
+        }


Ini bakal jadi weird behavior klo lu movieId missing ngepaksa user balik ke controller sebelumnya, ini bakal dikira ada bug. ada baiknya kalau emang missing yang lu bikin layout errornya jangan di force kyk gini.

zmkhtr · 2022-10-20T13:50:11Z

Movie-App-Group-7/Movie-App-Group-7/UI/Detail Movie/DetailMovieViewController.swift

+        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)
+                }
+            }
+        )
+
+        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)
+                }
+            }
+        )


ini sebenernya bisa lu rubah jadi satu fungsi tinggal dirubah parameter imageViewnya aja.

Suggested change

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)

}

}

)

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)

}

}

)

private func loadImageUsingKingfisher(url: URL, urlPoster: URL) {

downloadImage(for : movieImageContainerView, with : url)

downloadImage(for : movieBannerImageView, with : urlPoster)

}

private func downloadImage(for imageView: UIImageView, with url: URL) {

isDownloadingImage(true, imageView)

imageView.kf.setImage(

with: url,

placeholder: nil,

options: nil,

progressBlock: { [weak self] _, _ in

self?.isDownloadingImage(true, imageView)

},

completionHandler: { [weak self] result in

switch result {

case .success(_):

self?.isDownloadingImage(false, imageView)

case .failure(let error):

self?.isDownloadingImage(false, imageView)

print(error)

}

}

)

}

zmkhtr · 2022-10-20T13:52:03Z

Movie-App-Group-7/Movie-App-Group-7/UI/Home/TableViewCell/MovieTableViewCell.swift

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


ini juga sama, klo lu ngerasa berulang bgt, bisa dibikin ImageLoader nya sendiri jadi tinggal manggil class itu

zmkhtr · 2022-10-20T13:52:55Z

Movie-App-Group-7/Movie-App-Group-7/UI/Login/LoginViewController.swift

+        alert.addAction(UIAlertAction(title: "Ok", style: .default, handler: { action in
+            switch action.style {
+            case .default:
+                print("default")
+
+            case .cancel:
+                print("cancel")
+
+            case .destructive:
+                print("destructive")
+
+            default:
+                print("default")
+            }
+        }))


Klo gaada action yg dihandle ga perlu diimplement switch case nya

zmkhtr · 2022-10-20T14:00:02Z

Movie-App-Group-7/Movie-App-Group-7/UI/Detail Movie/ViewModel/DetailMovieViewModel.swift

+class DetailMovieViewModel {
+    private let movieNetworkModel : MovieNetworkModel
+
+    init(movieNetworkModel: MovieNetworkModel) {
+        self.movieNetworkModel = movieNetworkModel
+    }
+
+    func retrieveMovie(movieId: String, completion : @escaping (RetrievingDetailMovieState) -> ()) {
+        movieNetworkModel.getMovie(movieId: movieId) { result in
+            switch result {
+            case .success(let movie):
+                completion(.success(movie))
+            case .failure(let message):
+                completion(.failure(message))
+            }
+        }
+
+    }
+}


Klo niat lu implement MVVM, lu gabisa bilang ini ViewModel, karena viewModel di MVVM itu punya binder/observer yang menjadi pembeda itu sama MVC. lu harus bikin binder/observernya terus di observer di view, instead of lu handle from escaping when calling retrieveMovie(..). jadi Klo MVVM gamake result dari Completion ini jadi ga perlu ada completion di function retrieveMovie(..).

Ini berlaku untuk semua ViewModel yang lu implementasikan Project ini

Suggested change

class DetailMovieViewModel {

private let movieNetworkModel : MovieNetworkModel

init(movieNetworkModel: MovieNetworkModel) {

self.movieNetworkModel = movieNetworkModel

}

func retrieveMovie(movieId: String, completion : @escaping (RetrievingDetailMovieState) -> ()) {

movieNetworkModel.getMovie(movieId: movieId) { result in

switch result {

case .success(let movie):

completion(.success(movie))

case .failure(let message):

completion(.failure(message))

}

}

}

}

class DetailMovieViewModel {

private let movieNetworkModel : MovieNetworkModel

private var resultsObserver : ((Movie) -> ()?) // nanti ini yg lu observer di Controllernya

private var errorObserver : ((String) -> ()?) // nanti ini yg lu observer di Controllernya

init(movieNetworkModel: MovieNetworkModel) {

self.movieNetworkModel = movieNetworkModel

}

func retrieveMovie(movieId: String) {

movieNetworkModel.getMovie(movieId: movieId) { result in

switch result {

case .success(let movie):

resultsObserver?(movie)

case .failure(let message):

errorObserver?(message)

}

}

}

}

Fostahh added 3 commits October 19, 2022 00:28

Implement Login, Profile, and ListMovie

d8dd6b0

Implement Detail Movie

b9dcc29

Implementing API Call and Check if user logged in or not

792a84c

Fostahh force-pushed the second-task branch 4 times, most recently from e0a4404 to 3da7100 Compare October 20, 2022 09:04

Implement Shimmering effect when loading image

994b280

Fostahh force-pushed the second-task branch from 3da7100 to 994b280 Compare October 20, 2022 09:05

zmkhtr requested changes Oct 20, 2022

View reviewed changes

Fixing based on mentor suggestions

82dc601

Fostahh force-pushed the second-task branch from 2892a68 to 82dc601 Compare October 21, 2022 02:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ghibli Movie APp #4

Ghibli Movie APp #4

Fostahh commented Oct 20, 2022

zmkhtr left a comment

zmkhtr Oct 20, 2022

Fostahh Oct 21, 2022

zmkhtr Oct 20, 2022

zmkhtr Oct 20, 2022

zmkhtr Oct 20, 2022

Fostahh Oct 21, 2022

zmkhtr Oct 20, 2022

zmkhtr Oct 20, 2022

zmkhtr Oct 20, 2022

zmkhtr Oct 20, 2022

zmkhtr Oct 20, 2022

Ghibli Movie APp #4

Are you sure you want to change the base?

Ghibli Movie APp #4

Conversation

Fostahh commented Oct 20, 2022

zmkhtr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment