-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
일기장 [Step3] RedMango, Minsup #141
base: ic_9_redmango
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
셀 내 이미지가 간헐적으로 보였다 안보였다 하는데, 이부분 수정이 필요해보입니다~!
날씨와 관련된 네트워크 통신만 할게 확실한데 시간이 부족한 상황에서 네트워크 매니저에 확장성이나 유지보수성을 신경써야하는가
지금은 학습을 위한 프로젝트라 비교적 스텝이나 요구사항이 확실하고 제한되어있다고 생각하실 수 있는데요, 실제 회사에서 업무를 하게 되신다면 어떨까요? 요구사항이 절대 바뀌지 않을거라고 확신할 수 있을지 모르겠습니다. 왜 유연한 코드를 작성해야한다고 생각하세요? 왜 SOLID 원칙이 중요하다고 하는걸까요? 왜 객체지향적으로 코드를 짜는게 필요하다고 생각하세요? 레드망고와 민섭의 생각이 궁금합니다.
struct WeatherDecoder { | ||
static func decode(jsonData: Data) throws -> Weather? { | ||
do { | ||
let decoder = JSONDecoder() | ||
let weatherData = try decoder.decode(WeatherData.self, from: jsonData) | ||
|
||
return weatherData.weather.first | ||
} catch { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WeatherData
라는 타입에 종속적이지 않게 만들 수도 있지 않을까요?
import Foundation | ||
|
||
enum WeatherDecodingError: Error { | ||
case decodeFail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decoding이 fail하는 게 꼭 Weather라는 타입과 연관 있지는 않을 것 같아요
struct WeatherData: Decodable { | ||
let weather: [Weather] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weather의 배열타입이라 변수명은 복수형태가 어떨까요?
func fetch(_ completion: @escaping (Data?) -> Void) { | ||
locationManager.fetchSingleLocation { location in | ||
Task { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
location을 전달받을 때 꼭 클로저로 전달받아야 하나요?
do { | ||
let weather = try await NetworkManager.fetchCurrentWeather(coordinate: location.coordinate) | ||
|
||
let icon = try await NetworkManager.fetchWeatherIcon(icon: weather?.icon) | ||
completion(icon) | ||
} catch { | ||
completion(nil) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NetworkManager 내의 fetch~ 메서드들은 throws로 에러를 던지고있는데,
여기선 관련한 처리는 없고 실패하면 그냥 nil을 전달할 뿐이네요.
실패했을 때 어떻게 핸들링하면 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고 해당 객체는 WeatherFetcher
인데, 정작 날씨 관련 정보를 가지고 오는 건 NetworkManager
내부의 메서드가 하는 것 같아요.
어떤 기준으로 해당 객체들의 책임을 나누셨나요?
struct WeatherFetcher { | ||
let locationManager: LocationManager | ||
|
||
init(locationManager: LocationManager = LocationManager()) { | ||
self.locationManager = locationManager | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WeatherFetcher
내에서 현재의 경,위도 정보만 필요한거라면
해당 값만 주입받는 방법은 어떨까요?
LocationManager
객체 자체를 들고있기엔 현재 구조체의 기능이나 책임이 조금 모호하게 비대한 느낌이 들어요
self.locationCompletion = completion | ||
|
||
locationManager.requestWhenInUseAuthorization() | ||
locationManager.startUpdatingLocation() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self를 붙이고 안붙이고의 기준이 무엇인가요?
@delmaSong 안녕하세요 delma!!!!
이번 주가 프로젝트 마지막이여서 밤을 새가며 완성을 했습니다...
많이 부족하고 복잡하지만 노력많이했습니다🥲
많은 피드백 부탁드립니다. 감사합니다!!
고민 했던 점
1️⃣ 날씨와 관련된 네트워크 통신만 할게 확실한데 시간이 부족한 상황에서 네트워크 매니저에 확장성이나 유지보수성을 신경써야하는가
2️⃣ 일기장을 저장하는 순간 네트워크가 불안할 경우 어떤 처리를 해야할지
일기를 저장하는 순간에 네트워크가 불안해서 아이콘 불러오기를 실패하면 어떻게 처리해야할 지 기획을 했습니다.
이전에는 날씨아이콘 id를 저장해서 계속 네트워크 통신을 했었지만 지금은 이미지를 통채로 저장해서 네트워크 통신을 계속 하지 않게 끔 기획적으로 변경하였습니다.
이전 날씨를 가져오는 API를 이용해 이미지를 복구하는 작업은 추후에 도전해보겠습니다.
해결하지 못한 점
이미지 크기 조절 실패
정적으로 24값을 주지 않고 옆에있는 CreatedDateLabel의 높이와 같게 하고 싶었으나.. createdDateLabel이 커져버리는 현상이 생겼습니다.. 어떤 것을 고려해봐야할까요...