날씨 앱 [STEP 1] eunduk2 #38

EunDuk2 · 2024-04-21T14:18:24Z

@havilog
1차적으로 할 수 있는데까지 해보았습니다!

WeatherViewController와 WeatherDetailViewController를 view와 controller로 나누었습니다.
많은 일을 하는 메서드들을 분리하였습니다.
WeatherViewController가 갖는 WeatherDetailViewController의존성을 DIP하여 구현하였습니다.

havilog

구현하느라 고생하셨어요!
리뷰 착각해서 죄송합니다 ㅜㅜ

havilog · 2024-04-22T13:21:44Z

WeatherForecast/WeatherForecast/SceneDelegate.swift

@@ -15,7 +15,20 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
        // Use this method to optionally configure and attach the UIWindow `window` to the provided UIWindowScene `scene`.
        // If using a storyboard, the `window` property will automatically be initialized and attached to the scene.
        // This delegate does not imply the connecting scene or session are new (see `application:configurationForConnectingSceneSession` instead).
-        guard let _ = (scene as? UIWindowScene) else { return }
+//        guard let _ = (scene as? UIWindowScene) else { return }


주석은 지워주세요!

havilog · 2024-04-22T13:22:24Z

WeatherForecast/WeatherForecast/SceneDelegate.swift

+        navigationController.navigationBar.prefersLargeTitles = true
+        navigationController.navigationBar.tintColor = .black


외부에서 프로퍼티를 변경하는것은 지양하면 좋을거 같아요~

navigationController는 UIKit에 포함되어 있는데 외부에서 변경하지 않는 방법이 있을까요??

WeatherViewController에서 self.navigationController에 접근해서 viewDidLoad 시점에 변경시켜주는게 좋아보여요!

havilog · 2024-04-22T13:22:53Z

WeatherForecast/WeatherForecast/WeatherDetailView.swift

+import Foundation
+import UIKit


파운데이션은 유킷 안에 들어있어서 유킷만 임포트 해주시면 좋을거 같아요!

havilog · 2024-04-22T13:23:46Z

WeatherForecast/WeatherForecast/WeatherDetailView.swift

+protocol WeatherDetailViewDelegate {
+    func setValue(weatherGroupLabel: UILabel, weatherDescriptionLabel: UILabel, temperatureLabel: UILabel, feelsLikeLabel: UILabel, maximumTemperatureLable: UILabel, minimumTemperatureLable: UILabel, popLabel: UILabel, humidityLabel: UILabel, sunriseTimeLabel: UILabel, sunsetTimeLabel: UILabel, iconImageView: UIImageView)
+}


WeaderView안에 nested protocol로 작성해서
WeaderView.Delegate로 작성해주시면 읽기 더 편할거 같아요!

havilog · 2024-04-22T13:24:22Z

WeatherForecast/WeatherForecast/WeatherDetailView.swift

+    private let sunsetTimeLabel: UILabel = UILabel()
+    private let spacingView: UIView = UIView()
+
+    private var delegate: WeatherDetailViewDelegate


delegate는 대부분 weak로 잡아주시면 좋을거 같아요!

havilog · 2024-04-24T02:57:40Z

WeatherForecast/WeatherForecast/WeatherDetailViewController.swift

+    private func setIconImage(icon:String, iconImageView: UIImageView) {
+        Task {
+            let urlString: String = "https://openweathermap.org/img/wn/\(icon)@2x.png"
+
+            guard let url: URL = URL(string: urlString),
+                  let (data, _) = try? await URLSession.shared.data(from: url),
+                  let image: UIImage = UIImage(data: data) else {
+                return
+            }
+
+            iconImageView.image = image
        }


이미지를 다운로드 하는 객체
이미지를 캐시하는 객체
캐시이미지를 가져올지 리모트 이미지를 가져올지 판단하는 객체
등으로 분리해보는 건 어떨까요?

havilog · 2024-04-24T02:59:36Z

WeatherForecast/WeatherForecast/WeatherDetailViewController.swift

+    }
+
+    // protocol function
+    func setValue(weatherGroupLabel: UILabel, weatherDescriptionLabel: UILabel, temperatureLabel: UILabel, feelsLikeLabel: UILabel, maximumTemperatureLable: UILabel, minimumTemperatureLable: UILabel, popLabel: UILabel, humidityLabel: UILabel, sunriseTimeLabel: UILabel, sunsetTimeLabel: UILabel, iconImageView: UIImageView) {


label들을 이렇게 파라미터로 받는건 지양해야할거 같아요!
또한 파라미터가 4개 이상이면 리팩토링을 고민해보시면 좋을거 같아요

label을 파라미터로 받는 것을 지양해야 하는 이유는 캡슐화를 위반해서 일까요??
그렇다면 제가 구현한 프로토콜 함수중에 파라미터로 tableView랑 indexPath를 넘겨준 것도 지양해야 할 까요?

넵 각 객체들이 소유하고 있는 "프로퍼티"는 외부에서는 몰라야하고, 자신만 알고 있어야해요!
프로퍼티의 값 혹은 이벤트를 전달해야합니다!
극단적으로 말해서 UILabel, UIImage 등을 파라미터로 넘기는 함수는 없다고 생각하시면 편할거 같아요!

havilog · 2024-04-24T03:01:13Z

WeatherForecast/WeatherForecast/WeatherViewController.swift

+
+    override func loadView() {
+        view = WeatherView(delegate: self)
+        refresh()


로드 뷰 시점에는 뷰를 갈아끼우는 것 외에 다른 동작은 지양해야할거 같아요!
이 동작은 로드 이후에 해야할거 같아요

havilog · 2024-04-24T03:02:05Z

WeatherForecast/WeatherForecast/WeatherViewController.swift

+        navigationItem.rightBarButtonItem = buttonItem
+    }
+
+    func changeTempUnit() {


tempUnit에 토글을 만들어보눈건 어떨까요?

havilog · 2024-04-24T03:03:29Z

WeatherForecast/WeatherForecast/WeatherViewController.swift

+        return data
+    }
+
+    private func decodeWeatherJSON(dataName: String) -> WeatherJSON? {


제이슨을 받아오는 객체도 분리되면 좋을거 같아요

EunDuk2 · 2024-04-26T15:00:54Z

@havilog

시간이 충분하지 않아서 주신 피드백의 일부와 Step2 OCP기능만 수정해보았습니다!

havilog

3주간 고생많으셨습니다!!

havilog · 2024-04-28T03:57:29Z

WeatherForecast/WeatherForecast/ImageManager.swift

+struct ImageManager {
+    private let imageChache: NSCache<NSString, UIImage> = NSCache()
+
+    func downloadImage(urlString: String) async -> UIImage? {
+        guard let url: URL = URL(string: urlString),
+              let (data, _) = try? await URLSession.shared.data(from: url),
+              let image: UIImage = UIImage(data: data) else {
+            return nil
+        }
+        return image
+    }
+
+    func getImageChache(forKey: String) -> UIImage? {
+        guard let image = imageChache.object(forKey: forKey as NSString) else { return nil }
+        return image
+    }
+
+    func setImageChache(image: UIImage, forKey: String) {
+        imageChache.setObject(image, forKey: forKey as NSString)
+    }
+}


ImageCachable 과 같은 프로토콜을 선언하고, 이미지 캐시만을 담당하는 객체를 만들어서 이미지 다운로드 하는 객체와 분리해보는건 어떨까요?

havilog · 2024-04-28T03:58:14Z

WeatherForecast/WeatherForecast/SceneDelegate.swift

+        navigationController.navigationBar.prefersLargeTitles = true
+        navigationController.navigationBar.tintColor = .black


WeatherViewController에서 self.navigationController에 접근해서 viewDidLoad 시점에 변경시켜주는게 좋아보여요!

havilog · 2024-04-28T04:01:43Z

WeatherForecast/WeatherForecast/WeatherDetailViewController.swift

+    }
+
+    // protocol function
+    func setValue(weatherGroupLabel: UILabel, weatherDescriptionLabel: UILabel, temperatureLabel: UILabel, feelsLikeLabel: UILabel, maximumTemperatureLable: UILabel, minimumTemperatureLable: UILabel, popLabel: UILabel, humidityLabel: UILabel, sunriseTimeLabel: UILabel, sunsetTimeLabel: UILabel, iconImageView: UIImageView) {


넵 각 객체들이 소유하고 있는 "프로퍼티"는 외부에서는 몰라야하고, 자신만 알고 있어야해요!
프로퍼티의 값 혹은 이벤트를 전달해야합니다!
극단적으로 말해서 UILabel, UIImage 등을 파라미터로 넘기는 함수는 없다고 생각하시면 편할거 같아요!

havilog · 2024-04-28T04:02:42Z

WeatherForecast/WeatherForecast/WeatherDetailViewController.swift

+        temperatureLabel.text = "현재 기온 : \(temperature.getTemperature(temp: listInfo.main.temp))\(temperature.unit)"
+        feelsLikeLabel.text = "체감 기온 : \(listInfo.main.feelsLike)\(temperature.unit)"
+        maximumTemperatureLable.text = "최고 기온 : \(listInfo.main.tempMax)\(temperature.unit)"
+        minimumTemperatureLable.text = "최저 기온 : \(listInfo.main.tempMin)\(temperature.unit)"
+        popLabel.text = "강수 확률 : \(listInfo.main.pop * 100)%"
+        humidityLabel.text = "습도 : \(listInfo.main.humidity)%"
+
+        if let cityInfo {
+            sunriseTimeLabel.text = "일출 : \(cityDateFormatter().string(from: Date(timeIntervalSince1970: cityInfo.sunrise)))"
+            sunsetTimeLabel.text = "일몰 : \(cityDateFormatter().string(from: Date(timeIntervalSince1970: cityInfo.sunset)))"
+        }


이 text를 포매팅하는 것도 객체로 분리할 수 있으면 좋을거 같네요!

EunDuk2 and others added 6 commits April 16, 2024 23:03

view, controller 1차 분리 작업

1125d8b

ViewController를 view와 controller로 분리

4afdca0

fetch함수 1차 리팩토링

ec2f80c

setCell함수 기능분리

7b75c13

WeatherDetailViewController -> view와 controller로 분리

9875be8

WeatherDetailViewController, DIP 적용

2f943ec

havilog reviewed Apr 24, 2024

View reviewed changes

eunsung added 7 commits April 25, 2024 20:55

사용하지 않는 코드 삭제

d8620a5

import foundation, uikit와 중복되므로 삭제

4004ec4

delegate -> weak로 선언

ff7d44f

imageManager생성 후 적용

1199e75

loadView에서는 view를 load하는 동작만 수행하도록 수정

5cc3f1c

changeTempUnit 토글형태로 수정

3a9868b

changeTempUnit 토글로 동작하게 수정 및 OCP 적용하여 화씨 섭씨 기능 구현

fd07a9b

havilog approved these changes Apr 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

날씨 앱 [STEP 1] eunduk2 #38

날씨 앱 [STEP 1] eunduk2 #38

EunDuk2 commented Apr 21, 2024

havilog left a comment

havilog Apr 22, 2024

havilog Apr 22, 2024

EunDuk2 Apr 26, 2024

havilog Apr 28, 2024

havilog Apr 22, 2024

havilog Apr 22, 2024

havilog Apr 22, 2024

havilog Apr 24, 2024

havilog Apr 24, 2024

EunDuk2 Apr 26, 2024

havilog Apr 28, 2024

havilog Apr 24, 2024

havilog Apr 24, 2024

havilog Apr 24, 2024

EunDuk2 commented Apr 26, 2024

havilog left a comment

havilog Apr 28, 2024

havilog Apr 28, 2024

havilog Apr 28, 2024

havilog Apr 28, 2024

		navigationController.navigationBar.prefersLargeTitles = true
		navigationController.navigationBar.tintColor = .black

		import Foundation
		import UIKit

날씨 앱 [STEP 1] eunduk2 #38

Are you sure you want to change the base?

날씨 앱 [STEP 1] eunduk2 #38

Conversation

EunDuk2 commented Apr 21, 2024

havilog 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EunDuk2 commented Apr 26, 2024

havilog 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