-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat [#140] 로그아웃, 회원탈퇴 액션 함수 추가 #141
Conversation
@@ -99,6 +99,7 @@ enum ImageLiterals { | |||
static var btnAccount: UIImage { .load(name: "btn_acoount") } | |||
static var btnFeedback: UIImage { .load(name: "btn_feedback") } | |||
static var btnCustomerCenter: UIImage { .load(name: "btn_customerCenter") } | |||
static var btnLogout: UIImage { .load(name: "btn_logout") } | |||
} | |||
|
|||
enum TransparencyInfo { |
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.
이 코드 패치는 이미지 리터럴 열거형에 "btnLogout" 이미지를 추가하는 변경 사항입니다.
버그의 위험성:
- 이 코드 패치에서는 명확한 버그 위험은 없습니다. 다만, 실제 프로젝트에 대한 정보가 없으므로 해당 이미지 파일들이 실제로 존재하고 올바르게 로드되는지 확인해야 합니다.
개선 제안:
- 이미지 리터럴을 사용할 때 확장자를 포함하여 정확한 이미지 이름을 사용하는 것이 좋습니다. 예를 들어, "btn_logout.png"와 같이 이미지 파일의 확장자를 명시하는 것이 좋습니다.
- 이미지 리터럴 메서드인 ".load(name:)"에 대한 구현이 필요합니다. 해당 메서드는 이미지 파일을 로드하는 로직을 담당해야 합니다.
코드 리뷰 완료했습니다. 추가 질문이 있으시면 답변해드리겠습니다!
@@ -141,6 +144,7 @@ enum StringLiterals { | |||
static let accountInfo = "계정 정보" | |||
static let feedback = "피드백 남기기" | |||
static let customerCenter = "고객센터" | |||
static let logout = "로그아웃" | |||
} | |||
|
|||
enum Network { |
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.
이 코드 패치는 몇 가지 변경사항을 포함하고 있습니다. 아래에서 각 변경사항에 대한 간략한 설명과 버그 위험 및 개선 제안을 제공하겠습니다:
- StringLiterals 열거형:
-
myPageMoreInfoTitle: 이용약관의 제목 문자열입니다.
- 버그 위험: 값이 주어지지 않은 경우 런타임 오류가 발생할 수 있습니다. 값을 확인하는 조건 또는 기본값 처리를 추가하는 것이 좋습니다.
-
myPageMoreInfoButtonTitle: "자세히 보기" 버튼의 제목 문자열입니다.
- 개선 제안: 문자열의 의미를 명확히 하는 것이 도움이 될 수 있습니다. 예를 들어, "이용약관 보기"와 같은 구체적인 제목으로 변경하면 사용자에게 더욱 명확한 정보를 제공할 수 있습니다.
-
myPageSignOutButtonTitle: "회원탈퇴" 버튼의 제목 문자열입니다.
- 개선 제안: 해당 기능을 사용하기 전에 사용자에게 경고 메시지를 표시하거나 추가 확인 단계를 추가하는 것이 좋습니다. 이렇게 하면 사용자가 실수로 버튼을 눌러 중요한 작업을 수행하는 상황을 방지할 수 있습니다.
- TransparencyInfo 열거형:
- logout: "로그아웃"을 나타내는 문자열입니다.
- 개선 제안: 대부분의 앱에서 로그아웃 기능은 중요하기 때문에 이용자에게 명확한 정보를 제공해야 합니다. 예를 들어, "로그아웃" 버튼의 옆에 사용자 이름을 표시하여 현재 로그인된 사용자를 보다 명확히 식별할 수 있도록 할 수 있습니다.
- Network 열거형: 이곳에서 변화는 없는 것으로 보입니다. 따라서 추가적인 개선 제안은 필요하지 않습니다.
추가로, 코드의 다른 부분에 대해서는 볼 수 없어서 더 자세한 리뷰를 제공할 수 없습니다. 위에서 언급한 변경사항을 고려하여 코드를 검토하고 수정하시면 됩니다.
"author" : "xcode", | ||
"version" : 1 | ||
} | ||
} |
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.
위의 코드 패치는 이미지 파일을 설명하는 JSON 형식의 데이터를 나타냅니다. 개선 제안 및 버그 위험에 대한 코드 검토를 진행해드리겠습니다.
-
"images" 배열에 있는 개별 이미지 객체들은 컴파일 시에 사용되는 이미지의 파일 이름, idiomatic 급식 명칭(Universal) 및 scale 정보를 포함하고 있습니다. 이 상황에서 문제점은 보이지 않습니다.
-
"info" 객체에는 작성자(author)와 버전(version) 정보가 포함되어 있으며, 이 또한 문제가 없어 보입니다.
개선 제안:
이 코드 패치는 이미지 파일과 관련된 정보를 표현하기 위한 간단한 JSON 형식을 사용하고 있습니다. 따라서 개선할 만한 부분은 크게 없어 보입니다. 그러나 추가적으로 고려할만한 사항 몇 가지를 제안드립니다:
-
이미지 파일의 경로나 기타 속성에 대한 유효성 검사를 수행하는 로직을 추가하는 것이 좋습니다.
-
이미지 크기에 관한 정보를 추가하여 다양한 화면 해상도에 대응할 수 있도록 하는 것이 유용할 수 있습니다.
-
코드 베이스 전체의 일관성을 유지하기 위해 JSON 파일에 대한 스키마 정의를 고려해볼 수도 있습니다. 이는 데이터 유효성 검사 및 문제 해결에 도움이 될 수 있습니다.
그러나 현재 상태에서는 주어진 코드 패치의 문제점은 보이지 않으며, 이미지 파일 정보를 표현하는 데에 충분합니다.
@@ -88,7 +88,7 @@ final class NetworkService: NetworkServiceType { | |||
return nil | |||
} else { | |||
guard let newAccessToken = result.data?.accessToken else { throw NetworkError.unknownError } | |||
KeychainWrapper.saveToken(accessToken, forKey: "accessToken") | |||
KeychainWrapper.saveToken(newAccessToken, forKey: "accessToken") | |||
|
|||
return try await donNetwork(type: type, baseURL: baseURL, accessToken: newAccessToken, body: body, pathVariables: pathVariables) | |||
} |
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.
이 코드 패치를 간단하게 검토해 드리겠습니다.
위의 코드 변경 사항은 accessToken 변수를 새로운 토큰(newAccessToken)으로 대체하는 것입니다. 이 변경은 보안 및 액세스 토큰 관련 문제에 몇 가지 개선을 제공할 수 있습니다. 이전 버전의 코드에서는 accessToken을 사용하여 저장하고 반환하였으나, 변경된 코드에서는 새로 생성된 액세스 토큰(newAccessToken) 값을 저장하고 사용합니다.
버그 리스크나 개선 제안은 주어진 코드 조각 외에는 확인할 수 없으므로, 더 자세한 정보가 있다면 추가로 제공해주시면 감사하겠습니다.
$0.centerX.equalToSuperview() | ||
$0.leading.trailing.equalToSuperview().inset(15.adjusted) | ||
$0.top.equalTo(customerCenterButton.snp.bottom).offset(10.adjusted) | ||
} | ||
} | ||
|
||
func showSettings() { |
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.
기능적인 측면에서 이 코드 패치에 문제점은 없어 보입니다. 그러나 몇 가지 개선 제안이 있습니다:
-
DontBeBottomSheetButton
클래스를 사용하는 모든 버튼 초기화에 대해 중복 코드를 만들고 있습니다. 여기서 인터페이스 빌더 또는 팩토리 메서드를 고려하여 이 코드 중복을 줄일 수 있습니다. -
버튼의 레이아웃 제약 조건을 설정하는 부분이 중복되어 있습니다.
setMultiButtonLayout()
메서드에서 이러한 중복을 줄일 수 있도록 변경할 수 있습니다. -
showSettings()
메서드는 구현되지 않았으므로 제거하는 것이 좋습니다. -
뷰의 높이가 하드 코딩되어 있으며, 대신 상대적인 값이 아닌 프레임과 관련된 제약 조건을 사용하면 더 일반화된 솔루션이 될 수 있습니다.
이러한 개선 사항을 고려하여 코드를 리팩토링할 수 있을 것으로 보입니다.
@objc | ||
private func signOutButtonTapped() { | ||
print("signOutButtonTapped") | ||
} | ||
} | ||
|
||
extension MyPageAccountInfoViewController: UITableViewDelegate { |
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.
다음은 코드 패치에 대한 간단한 코드 리뷰입니다:
-
moreInfoTitle
과moreInfoButton
,signOutButton
등의 속성이 정의되어 있습니다. 이러한 속성들은 코드에서 재사용될 수 있으므로, 공통으로 사용되는 값과 문자열을 상수 또는 열거형(enum)으로 추출하여 중복을 제거하는 것이 좋습니다. -
setAddTarget()
메소드에서signOutButton.addTarget(...)
를 추가하였습니다. 버튼에 대한 액션 연결은 올바르게 작동할 것으로 보입니다. -
signOutButtonTapped()
메소드가 추가되었습니다. 현재 구현 내용으로는 화면에 "signOutButtonTapped"을 출력하는 기능만 수행합니다. 실제 회원 탈퇴 로직을 구현해야 하는 경우, 해당 메소드 내에 추가하여야 합니다. -
이외의 변경 사항은 없어 보입니다.
개선 제안:
- 문자열 상수를 사용하여 반복되는 문자열 값을 중복없이 관리하세요.
- 리소스 파일을 사용하여 텍스트를 외부화하여 다국어 지원 및 유지보수 용이성을 높이세요.
- 적절한 에러 처리(log, exception 등)를 추가하여 예외 상황을 처리하세요.
rootView.myPageBottomsheet.handleDismiss() | ||
print("logoutButtonTapped") | ||
} | ||
|
||
@objc | ||
private func feedbackButtonTapped() { | ||
rootView.myPageBottomsheet.handleDismiss() |
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.
이 코드 패치에서는 MyPageViewController의 확장(extension) 부분을 변경하고 있습니다. main 작업은 다음과 같습니다:
-
217번 줄에서는 rootView의 myPageBottomsheet에 대한 버튼 타겟 설정을 수행하고 있습니다. 제공된 수정사항은 로그아웃 버튼(logoutButton)에 대한 타겟 설정을 추가하는 것입니다.
-
414번 줄에서는 logoutButtonTapped()이라는 새로운 메서드를 정의하고 있습니다. 이 메서드는 로그아웃 버튼이 탭됐을 때 호출되며, myPageBottomsheet가 해제되고 "logoutButtonTapped"이라는 로그가 출력됩니다.
개선할 점:
- 코드 자체는 주어진 요구사항을 충족시키고 있는 것으로 보이지만, 더 자세한 정보가 있다면 추가적인 사항을 알려주세요.
- 이 코드를 실제로 테스트하여 예상대로 작동하는지 확인하는 것이 좋습니다.
- 문제가 없다면, 되도록 코드 내용에 대한 설명 주석을 추가하는 것이 도움이 될 수 있습니다.
@@ -77,7 +77,8 @@ final class MyPageView: UIView { | |||
var myPageBottomsheet = DontBeBottomSheetView(profileEditImage: ImageLiterals.MyPage.btnEditProfile, | |||
accountInfoImage: ImageLiterals.MyPage.btnAccount, | |||
feedbackImage: ImageLiterals.MyPage.btnFeedback, | |||
customerCenterImage: ImageLiterals.MyPage.btnCustomerCenter) | |||
customerCenterImage: ImageLiterals.MyPage.btnCustomerCenter, | |||
logoutImage: ImageLiterals.MyPage.btnLogout) | |||
|
|||
var deleteBottomsheet = DontBeBottomSheetView(singleButtonImage: ImageLiterals.Posting.btnDelete) | |||
var warnBottomsheet = DontBeBottomSheetView(singleButtonImage: ImageLiterals.Posting.btnWarn) |
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.
@@ -77,7 +77,8 @@ 마지막 클래스 MyPageView 내에서 변경 사항이 있습니다:
myPageBottomsheet 변수에 logoutImage
매개변수를 추가했습니다:
var myPageBottomsheet = DontBeBottomSheetView(profileEditImage: ImageLiterals.MyPage.btnEditProfile,
accountInfoImage: ImageLiterals.MyPage.btnAccount,
feedbackImage: ImageLiterals.MyPage.btnFeedback,
customerCenterImage: ImageLiterals.MyPage.btnCustomerCenter,
logoutImage: ImageLiterals.MyPage.btnLogout)
deleteBottomsheet 변수에 singleButtonImage
매개변수를 추가했습니다:
var deleteBottomsheet = DontBeBottomSheetView(singleButtonImage: ImageLiterals.Posting.btnDelete)
warnBottomsheet 변수에 singleButtonImage
매개변수를 추가했습니다:
var warnBottomsheet = DontBeBottomSheetView(singleButtonImage: ImageLiterals.Posting.btnWarn)
전체적으로 보면 코드 변경은 안전한 것 같습니다.
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.
와우!! 로그아웃 구현 굿굿 고생했어요 ㅡ👍👏
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.
P5
우와아아 수고많으셨습니다 ㅠㅠ
👻 PULL REQUEST
💻 작업한 내용
💡 참고사항
로그아웃은 아래 코드에서 로직 처리하면 됩니다!
Don-tBe-iOS/DontBe-iOS/DontBe-iOS/Presentation/MyPage/ViewControllers/MyPageViewController.swift
Lines 418 to 422 in 50f3081
회원탈퇴는 아래 코드에서 로직 처리하면 됩니당!
Don-tBe-iOS/DontBe-iOS/DontBe-iOS/Presentation/MyPage/ViewControllers/MyPageAccountInfoViewController.swift
Lines 228 to 231 in 50f3081
📸 스크린샷
📟 관련 이슈