Step 4 - Jacob, Lina #31
Conversation
- openPage 메드에서 웹뷰가 현재 로딩중이면 동작하지 않도록함
| enum ErrorMessage: String { | ||
| case url = "입력한 주소가 올바른 형태가 아닙니다." | ||
| } |
There was a problem hiding this comment.
참고해서 다음 프로젝트에는 적용해보겠습니다! 감사합니다
| override func viewDidAppear(_ animated: Bool) { | ||
| super.viewDidAppear(animated) | ||
|
|
||
| // viewDidLoad()에서는 view가 아직 view hierarchy에 추가되지 않아서 alert을 present할 수 없다. | ||
| // viewDidApear()는 view가 view hierarchy에 추가된 후 호출되므로 alert을 present할 수 있다. | ||
| guard webView.load(favoriteWebPageURL: .google) else { | ||
| return showError(error: .url) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Types & IBActions & Methods | ||
| enum ErrorMessage: String { | ||
| case url = "입력한 주소가 올바른 형태가 아닙니다." | ||
| } | ||
|
|
||
| func showError(error: ErrorMessage) { | ||
| let errorAlert = UIAlertController(title: "Error!", message: error.rawValue, preferredStyle: .alert) | ||
| let ok = UIAlertAction(title: "확인", style: .default, handler: nil) | ||
| errorAlert.addAction(ok) | ||
|
|
||
| self.present(errorAlert, animated: false) | ||
| openPage(url: FavoriteWebPageURL.google.rawValue) |
There was a problem hiding this comment.
super.viewDidAppear()를 지우셨네요 혹시 어떤 이유가 있으실까요?
자식클래스는 부모클래스의 동작을 대치하는 것이 아니라 확장하는 형태 여야 할 것 같아요.
👨👩👧
| func addHttpsFront(of url: String) -> String { | ||
| return "https://" + url | ||
| } | ||
|
|
||
| func checkFront(of url: String?) -> Bool { |
There was a problem hiding this comment.
메서드들이 정의되는 순서도 의도를 내포할 수 있습니다.
혹시 메서드의 정의 순서를 어떤 기준으로 매기고 계신가요? 😉
There was a problem hiding this comment.
이번에 리팩토링하면서 함께 고민했던 부분인데요!
원래는 openPage와 showError부분이 viewDidAppear 밑에 순서대로 나오고,
나머지는 같은 순서였어요.
그런데 이번에 리팩토링 과정에서 IBAction은 이것들 끼리, 나머지 메소드들이 메소드들끼리 함께 묶여있는게 더 보기 좋을 것 같아서 밑으로 내려봤어요!
혹시 두 메소드가 다시 위쪽에 위치하는게 가독성에 더 좋을까요?
There was a problem hiding this comment.
어떤 의도였는 지 궁금했었어요😉
리나 말대로 관련있는 메소드들끼리 붙어있는 건 보기 좋을 거 같아요
관련있는 메소드들간의 순서는 어떻게 정의하면 좋을까요?
| guard !webView.isLoading else { | ||
| return | ||
| } |
There was a problem hiding this comment.
궁금한 것이 있습니다 🙋♂️
webView가 _www.naver.com_를 요청하고 로딩 중에
www.yagom.net 을 입력하면 뒤에 요청된 www.yagom.net 이 무시되야하나요?
There was a problem hiding this comment.
현재 로직상으로는 무시됩니다.
이 부분을 추가한건 로딩중에 사용자가 이동 버튼을 중복되서 계속 누르는 경우를 방지하고자 한 것인데,
또 다른 문제를 생각하지 못한 것 같습니다.
만약 인터넷 속도가 매우 느린 상황이라면 로딩이 느릴것이고,
유저는 다른 페이지로 이동하고 싶지만 버튼을 눌러도 이동하지 못하는 상황이 발생할 것 입니다.
이 부분에 대한 보완 방법은 다시 생각해보겠습니다.
There was a problem hiding this comment.
로딩중에 사용자가 이동 버튼을 중복되서 계속 누르는 경우를 방지하고자 한 것
이런 케이스를 미리 감안하시다니 멋지네요!
👍
| enum FavoriteWebPageURL: String { | ||
| case google = "https://www.google.com/" | ||
| case yagomDotNet = "https://yagom.net/" | ||
| case naver = "https://www.naver.com/" |
There was a problem hiding this comment.
enum을 이용해서 구현하신 이유가 있을까요? 😉
case 자체로보다는 rawValue를 위해 만들어진 타입 같아서 여쭤봅니답
There was a problem hiding this comment.
즐겨찾는 페이지가 1개가 아니라 추가될 수도 있다고 생각하고 사용했습니다.
원래는 즐겨찾는 페이지 enum값을 받아서 이동하는 메서드가 있었지만,
리팩토링 과정에서 삭제하게 되서 현재 enum이 rawValue 참조로만 쓰이게 되었습니다.
There was a problem hiding this comment.
아하 그런 히스토리가 있었군요
스펙이 바뀌었다면 코드도 같이 변경되도 좋을 것 같아요😉
|
@O-O-wl 리뷰해주신 내용 수정했습니다! 다시 코드 리뷰와 머지 부탁드려요 ㅎㅎ |
|
고생하셨습니다 제이콥, 리나! |
Step 4 : 주소 자동 변경