Conversation
SungMinCho-Kor
left a comment
There was a problem hiding this comment.
리뷰 요청 주셔서 열심히 봤습니다ㅎㅎ
UI 너무 이쁘게 잘 구현하신 것 같아요!
스유까지 사용하셔서 놀랐습니다👍
고생하셨습니다~~
리뷰의 대부분이 컨벤션에 관한 내용이에요 ㅎ
줄바꿈, 들여쓰기, 주석 등의 내용이 대부분입니다!
화이팅..!
| // | ||
| // Config.xcconfig | ||
| // SOPT_Week4_NetWork | ||
| // | ||
| // Created by 정정욱 on 11/2/24. | ||
| // | ||
|
|
||
| // Configuration settings file format documentation can be found at: | ||
| // https://help.apple.com/xcode/#/dev745c5c974 | ||
| BASE_URL = http:/$()/211.188.53.75:8080 |
There was a problem hiding this comment.
헉. Config 파일이 올라왔네요... gitignore에 추가해야 할 것 같아요!
There was a problem hiding this comment.
gitignore를 습관화하도록 하겠습니다. 잘 사용하지 않았던 거 같습니다 😂
| // | ||
| // AppDelegate.swift | ||
| // SOPT_Week4_NetWork | ||
| // | ||
| // Created by 정정욱 on 11/2/24. | ||
| // | ||
|
|
||
| import UIKit | ||
|
|
||
| @main | ||
| class AppDelegate: UIResponder, UIApplicationDelegate { | ||
|
|
||
|
|
||
|
|
||
| func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool { | ||
| // Override point for customization after application launch. | ||
| return true | ||
| } | ||
|
|
||
| // MARK: UISceneSession Lifecycle | ||
|
|
||
| func application(_ application: UIApplication, configurationForConnecting connectingSceneSession: UISceneSession, options: UIScene.ConnectionOptions) -> UISceneConfiguration { | ||
| // Called when a new scene session is being created. | ||
| // Use this method to select a configuration to create the new scene with. | ||
| return UISceneConfiguration(name: "Default Configuration", sessionRole: connectingSceneSession.role) | ||
| } | ||
|
|
||
| func application(_ application: UIApplication, didDiscardSceneSessions sceneSessions: Set<UISceneSession>) { | ||
| // Called when the user discards a scene session. | ||
| // If any sessions were discarded while the application was not running, this will be called shortly after application:didFinishLaunchingWithOptions. | ||
| // Use this method to release any resources that were specific to the discarded scenes, as they will not return. | ||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
기본으로 생성되는 파일 부분들을 정리해 볼 생각을 하지 못했네요
|
|
||
|
|
||
| func sceneDidDisconnect(_ scene: UIScene) { | ||
| // Called as the scene is being released by the system. | ||
| // This occurs shortly after the scene enters the background, or when its session is discarded. | ||
| // Release any resources associated with this scene that can be re-created the next time the scene connects. | ||
| // The scene may re-connect later, as its session was not necessarily discarded (see `application:didDiscardSceneSessions` instead). | ||
| } | ||
|
|
||
| func sceneDidBecomeActive(_ scene: UIScene) { | ||
| // Called when the scene has moved from an inactive state to an active state. | ||
| // Use this method to restart any tasks that were paused (or not yet started) when the scene was inactive. | ||
| } | ||
|
|
||
| func sceneWillResignActive(_ scene: UIScene) { | ||
| // Called when the scene will move from an active state to an inactive state. | ||
| // This may occur due to temporary interruptions (ex. an incoming phone call). | ||
| } | ||
|
|
||
| func sceneWillEnterForeground(_ scene: UIScene) { | ||
| // Called as the scene transitions from the background to the foreground. | ||
| // Use this method to undo the changes made on entering the background. | ||
| } | ||
|
|
||
| func sceneDidEnterBackground(_ scene: UIScene) { | ||
| // Called as the scene transitions from the foreground to the background. | ||
| // Use this method to save data, release shared resources, and store enough scene-specific state information | ||
| // to restore the scene back to its current state. | ||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
보통 App, SceneDelegate 파일들의 주석을 지우시고 사용하지 않는 메서드도 삭제하시는 편인가요?
There was a problem hiding this comment.
PR에 올라가는 코드는 읽는 사람을 고려하여 최대한 코드를 정제해서 올리는 것이 좋다고 생각합니다!
그래서 다 정리해서 올립니다 ㅎ...
There was a problem hiding this comment.
역시 성민갓... 또하나 배워갑니다
|
|
||
|
|
There was a problem hiding this comment.
제가 습관적으로 항상 파일 끝에 줄 바꿈을 해두는 것 같습니다 메서드를 만들기 전에 }을 닫고 엔터를 두 번 치는 거 같아요…. 이유는 모르겠지만 뭔가 넓어진다는 느낌?
There was a problem hiding this comment.
ㅋㅋㅋㅋㅋㅋㅋ 넓어진다는 느낌 좋네요.
자신만의 컨벤션으로 하는 것이라면 모든 파일 끝에 줄바꿈을 두 번 하면 좋을 것 같아요.
|
|
||
|
|
||
|
|
||
| extension LoginViewController: UITextFieldDelegate { |
|
|
||
| // 실제 레이아웃 변경은 애니메이션으로 줄꺼야 | ||
| UIView.animate(withDuration: 0.3) { // 이코드가 없으면 애니메이션이 따로 없어서 조금 딱딱하게 보여짐 그래서 애니메이션 효과 주기위한 코드 | ||
| self.stackView.layoutIfNeeded() // 0.3 초동안 애니메이션 효과가 일어남 : 오토레이아웃 동적 조정할때는 이런 애니메이션 코드를 꼭 삽입함 |
There was a problem hiding this comment.
layoutIfNeeded는 무슨 함수이며 언제 호출하면 좋을까요?
There was a problem hiding this comment.
tackView의 레이아웃을 즉시 업데이트하도록 요청하는 메서드입니당 이 메서드는 뷰가 필요에 따라 레이아웃을 업데이트하는 과정을 기다리지 않고, 즉각적으로 레이아웃을 갱신하는 역할을 담당 하고 있어요!
언제 쓰면 좋은가?
애니메이션을 적용할 때: 레이아웃 업데이트를 애니메이션과 함께 자연스럽게 연결하고 싶을 때 유용합니다. 예를 들어, 특정 UI 요소의 크기나 위치가 변할 때 layoutIfNeeded()를 호출하면 해당 뷰가 즉시 업데이트되면서 애니메이션이 부드럽게 적용됩니다.
UIView.animate(withDuration: 0.3) {
self.stackView.layoutIfNeeded()
}| } | ||
|
|
||
| // MARK: - 이메일텍스트필드, 비밀번호 텍스트필드 두가지 다 채워져 있을때, 로그인 버튼 빨간색으로 변경 | ||
| @objc private func textFieldEditingChanged(_ textField: UITextField) { |
There was a problem hiding this comment.
UITextFieldDelegate에 입력을 컨트롤할 수 있는 함수도 있습니다.
참고해보세요~
공식 문서
| // | ||
| // ViewController.swift | ||
| // SOPT_Week4_NetWork | ||
| // | ||
| // Created by 정정욱 on 11/2/24. | ||
| // | ||
|
|
||
| import UIKit | ||
|
|
||
| class ViewController: UIViewController { | ||
|
|
||
| override func viewDidLoad() { | ||
| super.viewDidLoad() | ||
| // Do any additional setup after loading the view. | ||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
|
|
||
|
|
||
| var body: some View { | ||
|
|
jeonguk29
left a comment
There was a problem hiding this comment.
@SungMinCho-Kor 정성 가득한 코드 리뷰 감사합니다. 안 좋은 습관과 부족한 부분들을 많이 체크 했던거 같습니다. 이유 없이 무의식적으로 사용하던 코드들도 왜? 라는 질문을 던져보며 사용하도록 하겠습니다.
|
|
||
| import Foundation | ||
|
|
||
| struct RegisterRequest: Codable { |
There was a problem hiding this comment.
Codable 이 Encodable과 Decodable 프로토콜의 합성으로 구성되어 있어서 보통 Request, Response 구분을 하지 않고 사용하고 있습니다
|
|
||
|
|
There was a problem hiding this comment.
제가 습관적으로 항상 파일 끝에 줄 바꿈을 해두는 것 같습니다 메서드를 만들기 전에 }을 닫고 엔터를 두 번 치는 거 같아요…. 이유는 모르겠지만 뭔가 넓어진다는 느낌?
| import Foundation | ||
|
|
||
| enum Environment { | ||
| static let baseURL: String = Bundle.main.infoDictionary?["BASE_URL"] as! String |
There was a problem hiding this comment.
코드를 긁어 올 때 이런 부분 또한 확인하도록 하겠습니다.
| username: String, | ||
| password: String, | ||
| hobby: String, | ||
| completion: @escaping (Result<Bool, NetworkError>) -> Void |
There was a problem hiding this comment.
🧐 엄청 예리하시네요 저도 몰랐는데 completion(.success(true)), completion(.success(false)) 값을 실제로 활용하는 부분이 없습니다.
completion: @escaping (Result<Void, NetworkError>) -> Void 타임으로 수정해야 할거 같네요
| completion: @escaping (Result<Bool, NetworkError>) -> Void | ||
| ) { | ||
|
|
||
| /// baseURL + /user = http://211.188.53.75:8080/user |
There was a problem hiding this comment.
정말 회사에서 잘리기 좋네요. ㅠㅠ 감사합니다. 주석 확인, 보안 확인을 생활화하도록 하겠습니다.
| print("자동 로그인: 기존 토큰이 발견되었습니다.") | ||
|
|
||
| // 다음 페이지로 넘어가기 | ||
| let mcController = MainViewController() |
| override func viewWillAppear(_ animated: Bool) { | ||
| usernameTextField.text = "" | ||
| passwordTextField.text = "" | ||
| } |
There was a problem hiding this comment.
override func viewWillAppear(_ animated: Bool) {
super.viewWillAppear(animated) // 부모 클래스의 기본 동작 호출
usernameTextField.text = ""
passwordTextField.text = ""
}몰랐던 부분인데 부모 클래스가 viewWillAppear에서 설정한 기본 동작이 있기 때문에 기본 동작을 생략하면, 화면이 나타날 때 필요한 기본 설정이 누락될 수 있다고 하네요 감사합니다!
| } | ||
|
|
||
| // MARK: - 이메일텍스트필드, 비밀번호 텍스트필드 두가지 다 채워져 있을때, 로그인 버튼 빨간색으로 변경 | ||
| @objc private func textFieldEditingChanged(_ textField: UITextField) { |
| } | ||
|
|
||
| if textField == passwordTextField { // 유저가 선택한게 passwordTextField라면 | ||
| passwordTextFieldView.backgroundColor = #colorLiteral(red: 0.2972877622, green: 0.2973434925, blue: 0.297280401, alpha: 1) |
There was a problem hiding this comment.
감사합니다 리터럴이 좋지 않은 방법인거 같아요
|
|
||
| // 다음 페이지로 넘어가기 | ||
| let mcController = MainViewController() // MainViewController 인스턴스 생성 | ||
| self.navigationController?.pushViewController(mcController, animated: true) |
There was a problem hiding this comment.
클래스나 구조체 내부에서 자신의 인스턴스를 명확히 나타내기 위할때 사용하면 좋은거 같습니다!
choiyoubin
left a comment
There was a problem hiding this comment.
너무 멋진 코드 잘보고갑니다!!
SwiftUI도 오랜만에 보니 반갑네요... 사용한 이유도 풀리퀘본문에 적어주면 좋을것 같아요!!
코드 보고 로그아웃이나 다양한 부분들 많이 배워갑니다~ 고생하셨어요!!
| // | ||
| // Config.xcconfig | ||
| // SOPT_Week4_NetWork | ||
| // | ||
| // Created by 정정욱 on 11/2/24. | ||
| // | ||
|
|
||
| // Configuration settings file format documentation can be found at: | ||
| // https://help.apple.com/xcode/#/dev745c5c974 | ||
| BASE_URL = http:/$()/211.188.53.75:8080 |
| let result: Token | ||
| } | ||
|
|
||
| struct Token: Codable { |
There was a problem hiding this comment.
이름을 Upper과 lower로 구분해서 사용하면 헷갈리지는 않을까요?!
There was a problem hiding this comment.
제가 정말 네이밍을 소홀하게 여기는거 같습니다 😂 컨벤션 지켜보도록 하겠습니다
| completion: @escaping (Result<Bool, NetworkError>) -> Void | ||
| ) { | ||
|
|
||
| /// baseURL + /user = http://211.188.53.75:8080/user |
| // MARK: 내 취미 조회 | ||
| func fetchUserHobby(completion: @escaping (Result<String, NetworkError>) -> Void) { | ||
|
|
||
| guard let token = UserDefaults.standard.string(forKey: "userToken") else { |
There was a problem hiding this comment.
저도 UserDefaults로 관리하였는데, 빠르게 접근가능하고 키-값으로 데이터를 저장한다는 강점이 있어서 사용했는데, 나중에 실제로 프로젝트 및 진행한다면 저도 keychain 적용해 볼 예정입니다!
There was a problem hiding this comment.
UserDefaults을 Keychain바꾸면 추후에 보안성 면에서 더 좋겠지만 그 차이점을 같이 공부해서 나중에 이야기나눠봐요~(저는keychain은 사용안해봤어용ㅠㅠ)
There was a problem hiding this comment.
좋습니다 저도 키체인 사용해보지 않았지만 이번 합세때 적용해보시죠!
| } | ||
|
|
||
| // MARK: 내 취미 조회 | ||
| func fetchUserHobby(completion: @escaping (Result<String, NetworkError>) -> Void) { |
There was a problem hiding this comment.
조회 함수를 만드실때 대부분 fetch라는 함수명을 붙이시는 편일까요?
There was a problem hiding this comment.
네네 저는 주로 fetch를 붙이고 있습니다 더 좋은 네이밍이 있다면 추천해주세요!
| @SwiftUI.Environment(\.dismiss) var dismiss // 화면을 닫기 위한 dismiss 프로퍼티 | ||
|
|
||
| var body: some View { | ||
| VStack { |
| import Foundation | ||
| import Combine | ||
|
|
||
| final class HobbyViewModel: ObservableObject { |
There was a problem hiding this comment.
ViewModel을 따로 만들어서 사용하신 이유가 있을까요?! MVVM패턴을 사용하신건지도 궁금해요!
There was a problem hiding this comment.
맞습니다 사실 구조가 독특한데 스유 부분은 MVVM으로 구현이 되어 있습니다 요즘 현업에서는 유킷 + 스유 (TCA) 사용한다고 하네요
|
|
||
| // 💁 상속하지 않는다면 final 키워드 붙여주기 | ||
|
|
||
| final class LoginViewController: UIViewController { |
There was a problem hiding this comment.
오늘 말했던 저희의 합동세미나 컨벤션처럼 setStyle()함수안에 넣어서 .do를 이용해서 정리해봐도 좋을 것 같아요!
There was a problem hiding this comment.
진짜 같이 do it 해보시죠
| checkForExistingToken() | ||
| } | ||
|
|
||
| override func viewWillAppear(_ animated: Bool) { |
There was a problem hiding this comment.
맞습니다 로그아웃을 할때 남아 있는 부분을 제거 하기 위해서 썻는데 view will disappear 에 넣어도 괜찮을거 같아요
|
|
||
| override func viewDidLoad() { | ||
| super.viewDidLoad() | ||
| self.navigationItem.hidesBackButton = true |
There was a problem hiding this comment.
저는 navigationBar 관련된 값도 함수에 넣어서 설정하려고 하는데 나중에 같이고민해봐도 좋을 것 같아요~
There was a problem hiding this comment.
한사람이 짠거처럼.. 나연좌 가라사대
PR 가이드라인
PR Checklist
PR 날릴 때 체크 리스트
PR Type
어떤 종류의 PR인가요?
연관되는 issue 정보를 알려주세요
Issue Number: N/A
PR 설명하기
로그인 부분은 UIKit으로 그 외적인 부분은 SwiftUI로 구현하였습니다. 혹시 모를 솝커톤을 대비하기 위해 해당 방식으로 진행해 보았습니다.
가능하다면 추가해주세요
변경 사항 스크린샷 혹은 화면 녹화
기타 언급해야 할 사항들
어떤 것을 중점으로 리뷰 해주실 바라시나요?