부스트코스 “iOS 프로그래밍” 실습 프로젝트C. WeatherToday - 코드 리뷰
- Ref: 부스트코스 - iOS 프로그래밍
리뷰어에게 질문
Q. Segue로 데이터를 전달하기 구현 방법이 적절했는지?
A.
Segue로 데이터를 전달해주셨는데 전혀 틀린 방법이 아니고 권장하는 방법이 맞습니다.
근데 조금 더 개선하자면 전달하는 데이터를 struct하나로 만들어서 목적지 VC에서 초기화하는 것이 조금 더 깔끔한 방법입니다.
city나 country같이 가벼운 데이터는 그대로 넘겨도 무방하고, 좀 덩치가 큰 데이터는 필요한 데이터만 struct로 구성해서 넘기는 것을 권장합니다.
City.swift
getter 함수 형식보다는 Computed Property를 사용
날씨 state 정보가 숫자 코드로 되어있고 이것을 화면에 한국어로 표시하기위해서 함수로 만들어 코드값에 따라 날씨 정보를 반환하도록 했었다.
이때 enum에 Computed Property를 사용할 수 있는지 생각을 못했었다.
enum Weather: Int, Codable {
case sunny = 10
case cloudy = 11
case rainy = 12
case snowy = 13
func korean() -> String {
switch self {
case .sunny:
return "맑음"
case .cloudy:
return "흐림"
case .rainy:
return "비"
case .snowy:
return "눈"
}
}
}
리뷰 코멘트
#Advice 어떤 함수가 인자를 받는 것 없이 무언가 계산해서 돌려주는 getter 함수의 형식을 취하고 있다면 Computed property를 사용하는것이 좋습니다.
(단, 계산이 너무 오래걸리거나 Dynamic한 느낌을 주고 싶은 경우 함수의 형태를 취하는 경우도 많습니다.)
리뷰 적용
함수를 Computed Property로 수정
enum Weather: Int, Codable {
case sunny = 10
case cloudy = 11
case rainy = 12
case snowy = 13
var korean: String {
switch self {
case .sunny:
return "맑음"
case .cloudy:
return "흐림"
case .rainy:
return "비"
case .snowy:
return "눈"
}
}
}
Country.swift
Computed Property 활용
국기 이미지 에셋의 이름이 flag_국가명 형태로 되어있어서 이를 참조하기 위해 Computed Property를 사용했다.
struct Country: Codable {
let koreanName: String
let assetName: String
// 국기 이미지 에셋의 이름
var flagImageName: String {
return "flag_" + assetName
}
}
리뷰 코멘트
#Excellent
Computed Property를 잘 활용하신 것 같습니다.
잘하셨습니다! :D
BasicTypeExtension.swift
타입에 대해 범용적으로 쓰이지 못하는 Computed Property는 지양하자
섭씨를 화씨로 바꾸는 기능을 Float을 익스텐션해서 Computed Property로 구현했다.
extension Float {
var fahrenheitFromCelsius: Float {
return (self * 18.0).rounded() / 10.0 + 32.0
}
}
리뷰 코멘트
모든 Float에 대해 범용적으로 쓰이지 못하는 computed property는 지양하는 것이 좋습니다.
다음과 같이 타입을 만들어서 쓰는 것도 좋습니다.
struct Temparature {
let fahrenheit: Float
init(celsius: Float) {
self.fahrenheit = celsius * 1.8 + 32.0
}
init(fahrenheit: Float) {
self.fahrenheit = fahrenheit
}
var celsius: Float {
fahrenheit - 32.0 / 1.8
}
var textColor: UIColor {
switch celsius {
case ..<10.0:
return .blue
case 25.0...:
return .red
default:
return .black
}
}
}
리뷰 적용
Temparature 타입을 새로 만들고 JSON 디코딩을 위해 Codable 프로토콜을 적용
struct Temparature {
let celsius: Float
var fahrenheit: Float {
return (celsius * 18).rounded() / 10.0 + 32.0
}
var textColor: UIColor {
switch celsius {
case ..<10.0:
return .blue
case 25.0...:
return .red
default:
return .black
}
}
init(celsius: Float) {
self.celsius = celsius
}
init(fahrenheit: Float) {
self.celsius = fahrenheit - 32.0 / 1.8
}
}
extension Temparature: Codable {
init(from decoder: Decoder) throws {
celsius = try decoder.singleValueContainer().decode(Float.self)
}
}
LabelExtension.swift
함수의 이름은 명확하게
온도, 강수확률에 따라서 Label의 텍스트의 문구와 색상을 변경하는 기능이 필요했다. 다른 화면에서도 똑같은 코드가 필요해서 Label을 익스텐션한 함수를 만들어 재사용하게 했다.
extension UILabel {
/// Label을 온도 표시 양식으로 설정
/// - textColor: 25도 이상이면 빨간색, 10도 이하면 파란색
func setBy(celsius: Float) {
}
/// Label을 강수확률 표시 양식으로 설정
/// - textColor: 강수확률 60% 이상이면 오렌지색, 미만이면 검정색
func setBy(rainfallProbability: Int) {
}
리뷰 코멘트
#Advice 함수의 이름이 좀 더 명확했으면 좋겠어요.
setBy라고 하면 일단 어떤 하나의 대상에 대해 set하는 느낌이고 어떤 대상에 대해 set하는 것인지도 조금 모호해보여요.
updateText(withCelsius celsius: Float) 또는 configure(withCelsius celsius: Float)과 같은 네이밍은 어떨까요.
Float과 같이 Type만으로는 그 의미를 유추할 수 없는 경우 argumentLabel에 의미를 나타내는 명사를 함께 작성합니다. (with -> withCelsius)
To restore clarity, precede each weakly typed parameter with a noun describing its role
리뷰 적용
함수 이름을 updateText로 변경하고 argumentLabel로 기능 설명
extension UILabel {
/// Label을 온도 표시 양식으로 설정
/// - textColor: 25도 이상이면 빨간색, 10도 이하면 파란색
func updateText(withTemparature temparature: Temparature) {
}
/// Label을 강수확률 표시 양식으로 설정
/// - textColor: 강수확률 60% 이상이면 오렌지색, 미만이면 검정색
func updateText(withRainfallProbability rainfallProbability: Int) {
}
}
CityTableViewCell.swift
사용하지않는 override 메서드 제거
커스텀 테이블뷰 셀 클래스를 만들면 자동으로 생성되는 메서드가 있다.
class CityTableViewCell: UITableViewCell {
override func awakeFromNib() {
super.awakeFromNib()
// Initialization code
}
override func setSelected(_ selected: Bool, animated: Bool) {
super.setSelected(selected, animated: animated)
// Configure the view for the selected state
}
}
리뷰 코멘트
#Advice
override 메서드에서 부모의 메서드를 호출하는 것말고 따로 하는 일이 없다면 제거해도 무방합니다.
깔끔한 코드를 위해 이러한 함수들은 제거하는 것을 추천합니다.
리뷰 적용
사용하지 않는 overide 메서드 삭제
CityViewController.swift
identifier 용도의 상수는 Type Property로 하기
Segue identifier를 일반 상수로 선언했다.
class CityViewController: UIViewController {
let cityToDetailSegueIdentifier: String = "cityToDetail"
}
리뷰 코멘트
#Advice Identifier와 같은 상수는 ViewController의 인스턴스에 종속적인 것은 아니라서 일반적으로 static 으로 선언하여 사용하는 편입니다.
리뷰 적용
Segue Identifier를 Type Property로 변경
class CityViewController: UIViewController {
static let cityToDetailSegueIdentifier: String = "cityToDetail"
Property Observer로 초기화, 옵셔널 타입에 map 사용해서 코드 간결하게 하기
Segue로 city 정보를 전달할 하는 부분에서 내비게이션 바 타이틀을 도시의 이름으로 지정하는 코드가 같이 있다.
class CityViewController: UIViewController {
override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
// Get the new view controller using segue.destination.
// Pass the selected object to the new view controller.
guard let detailViewController: DetailViewController = segue.destination as? DetailViewController else {
print("DetailViewController 캐스팅 실패")
return
}
guard let city: City = sender as? City else {
print("City 캐스팅 실패")
return
}
// 내비게이션 바 타이틀을 선택한 셀의 도시 이름으로 지정
detailViewController.navigationItem.title = city.cityName
detailViewController.city = city
}
}
class DetailViewController: UIViewController {
var city: City?
}
리뷰 코멘트
#Advice
city를 이미 전달하고 있기 때문에, DetailViewController city의 didSet Property Observer를 이용해서 navigationItem.title을 지정할 수 있을 것 같습니다.
다음과 같이 Property Observer를 사용하여 초기화해볼 수 있습니다.
옵셔널 타입에 map을 사용하는 것은 코드를 간결하게 작성하는 한 가지 팁인데요, if let city = city { self.navigationItem.title = city.cityName } 과 동일한 의미입니다.
var city: City? {
didSet {
city.map { self.navigationItem.title = $0.cityName}
}
}
리뷰 적용
Property Obserber를 사용하여 내비게이션 바 타이틀 셋팅
class CityViewController: UIViewController {
override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
// Get the new view controller using segue.destination.
// Pass the selected object to the new view controller.
guard let detailViewController: DetailViewController = segue.destination as? DetailViewController else {
print("DetailViewController 캐스팅 실패")
return
}
guard let city: City = sender as? City else {
print("City 캐스팅 실패")
return
}
//detailViewController.navigationItem.title = city.cityName
detailViewController.city = city
}
}
class DetailViewController: UIViewController {
var city: City? {
didSet {
// 내비게이션 바 타이틀을 선택한 셀의 도시 이름으로 지정
city.map { self.navigationItem.title = $0.cityName}
}
}
}
Leave a comment