-
Notifications
You must be signed in to change notification settings - Fork 26
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
날씨앱 [STEP 1] caron #36
base: rft_2_caron
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.
안녕하세요. @Qussk !
날씨앱 프로젝트까지 진행해보셨네요!
Step1의 요구사항에 따라 분리해보려는 시도를 많이 해보셨네요!👍
질문주신 내용과 함께 코멘트를 적어보았습니다!
확인해보고 의견 나눠보면 좋을 것 같습니다!
protocol WeatherTableDelegate { | ||
func WeatherTableCell(cell: WeatherTableViewCell, indexPath: IndexPath, iconName: String, imageView: UIImageView) | ||
} |
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.
WeatherTableDelegate
프로토콜을 구현하신 이유가 있을까요?
class WeatherTableViewCell: UITableViewCell { | ||
var delegate: WeatherTableDelegate? |
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.
지금의 선언 형태에서 delegate
프로퍼티에 클래스 구체 타입이 할당 되었을 때, 메모리 이슈가 발생할 수 있지 않을까요?
현재 타입 내에서 delegate
프로퍼티를 사용하는 부분이 안보입니다! delegate 패턴에 대해 더 확인해보면 좋을 것 같습니다!
enum WeatherTitleType: String { | ||
case celsius = "섭씨" | ||
case fahrenheit = "화씨" | ||
} |
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.
온도 단위를 표현하는 타입으로 분리해보셨네요!👍
extension ViewController: WeatherTableDelegate { | ||
func WeatherTableCell(cell: WeatherTableViewCell, indexPath: IndexPath, iconName: String, imageView: UIImageView) { |
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.
기존의 tableView(_:cellForRowAt:)
에 작성되어 있던 로직을 새로운 메서드로 분리해보셨네요!
분리한 메서드에서도 다양한 일을 수행하는 것으로 보입니다. 각 역할을 파악해서 새로운 타입이나 메서드를 구현해보면 좋을 것 같습니다!
enum ImageURLType: String { | ||
case path = "https://openweathermap.org/img/wn/" | ||
case png = "@2x.png" | ||
} |
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.
URL String을 별도로 관리해보셨네요!👍
protocol WeatherForecastInfoDelegate { | ||
func weatherTask(iconName: String, imageView: UIImageView) | ||
} |
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.
WeatherForecastInfoDelegate
프로토콜을 구현하신 이유가 있을까요?
protocol dateFomatterSetUp { | ||
func dateSetUp(_ format: String?) -> DateFormatter | ||
} |
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.
지금과 같은 경우라면 프로토콜까지 구현하지 않아도 될 것 같습니다.
프로토콜을 사용해서 해결할 수 있는 문제를 더 알아보면 좋을 것 같습니다!
// //날짜형식 변환 | ||
// func dateSetUp(_ format: String?) -> DateFormatter { | ||
// let formatter: DateFormatter = DateFormatter() | ||
// let dateFormat = DateFormat(dataFormater: format, dateFormatStyle: .none) | ||
// formatter.timeStyle = .short | ||
// formatter.locale = .init(identifier: dateFormat.locale) | ||
// formatter.dateFormat = dateFormat.dataFormater | ||
// | ||
// guard format != nil else { | ||
// formatter.dateFormat = .none | ||
// return formatter | ||
// } | ||
// | ||
// return formatter | ||
// } |
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.
불필요한 주석은 제거해보는 것이 어떨까요?
class Utils { | ||
static func dateSetUp(_ format: String?) -> DateFormatter { |
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.
유틸성 기능을 별도의 타입과 메서드로 정의해보셨네요. 좋습니다!👍
지금처럼 다른 프로퍼티 없이 유틸리티 기능을 수행하는 부분에서는 static
메서드를 활용해도 좋아보입니다!
날씨앱 [STEP 2] 까지 했습니다!! 질문사항 : 리뷰 부탁드립니다! |
안녕하세요. SRP에 따라
날짜 formate에 따라 date형식을 변환하는 함수를 만들었습니다.
func dateSetUp(_ format: String?) -> DateFormatter
위 함수를 protocol로 만들어 ViewController와 WeatherDetailViewController에 모두 채택하여 사용하는게 좋은지
아니면,
Utils라는 클래스를 만들어서 static func로 만들어 사용하는 게 런타임 기준으로 뭐가 효율적인 건가요?…
둘다 정적으로 돌고있다고 봐도 되는건가요?
(제가 실무때 이런식으로 많이 썼었거든요 ㅠ)
**WeatherDetailViewController에서는 Utils라는 클래스를 활용하였고 ,
ViewController에서는 프로토콜을 채택하여 사용하였습니다.