-
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 [#105] 404 페이지 구현 및 답글에서 프로필 클릭시 유저페이지로 이동 및 유저 투명도에 따른 글쓰기 분기처리 #110
Conversation
@@ -1254,6 +1265,7 @@ | |||
2A2672022B4C3B44009D214F /* ViewModelType.swift in Sources */, | |||
3C6192ED2B3A719A00220CEB /* AppDelegate.swift in Sources */, | |||
2F8735402B4BE65300E55552 /* HomeView.swift in Sources */, | |||
2A5D68FC2B5938BD000AE1B7 /* ErrorViewController.swift in Sources */, | |||
2F8735442B4BE67300E55552 /* HomeViewModel.swift in Sources */, | |||
2F87354C2B4D28D700E55552 /* HomeCollectionFooterView.swift in Sources */, | |||
2A5220EC2B507F2A001510B7 /* UITableViewCellRegisterable.swift in Sources */, |
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.
주어진 코드 패치의 코드 리뷰를 해드리겠습니다.
- 빌드 파일과 파일 참조 추가:
ErrorViewController.swift
가 소스로 추가됩니다.
- 그룹 및 경로 추가:
404
이라는 이름의 새로운 그룹이 생성되며,ErrorViewController.swift
파일이 해당 그룹에 포함됩니다.
- 폴더 구조 변경:
404
그룹이Presentation
그룹 내에 추가됩니다.
- 파일 참조 추가:
ErrorViewController.swift in Sources
파일 참조가 추가됩니다.
버그나 개선점을 파악하기 위해서는 전체 소스 코드와 어떤 프레임워크/도구를 사용하는지 알아야 합니다. 따라서 여기서 설명드릴 수 있는 점은 제한적입니다. 이 코드 패치가 올바르게 작동하려면 다음을 고려해야 할 수 있습니다:
- 코드 변경의 목적과 예상 동작을 검토하세요.
- 파일 및 그룹 구조가 원하는 방식으로 설정되었는지 확인하세요.
- 코드 간의 의존성이 올바르게 구성되었는지 확인하세요.
- 파일을 올바른 위치와 그룹에 배치했는지 확인하세요.
- 필요한 빌드 설정이 적용되었는지 확인하세요.
- 다른 코드 변경 사항과 충돌이 발생하는지 주의하세요.
이외에도 세부적인 내용은 제공되지 않아서 더 자세한 코드 분석이 어렵습니다. 전체 코드를 주실 경우 보다 정확하고 구체적인 피드백을 제공할 수 있습니다.
@@ -13,6 +13,7 @@ enum ImageLiterals { | |||
static var logoSymbol: UIImage { .load(name: "logo_symbol") } | |||
static var imgProfile: UIImage { .load(name: "img_profile") } | |||
static var btnBackGray: UIImage { .load(name: "btn_back_gray") } | |||
static var img404: UIImage { .load(name: "img_404") } | |||
} | |||
|
|||
enum TabBar { |
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.
코드 수정 내용:
- 새로운 이미지 리터럴
img404
이 추가되었습니다.
버그 위험:
- 보안 상의 문제는 없어 보입니다. 그러나 코드에서 실제로 로드되는지 확인해야 합니다.
개선 제안:
- 이미지 리터럴을 사용하는 다른 부분과 일관성을 유지하기 위해
img404
대신에 더 명확한 이름을 선택하는 것이 좋습니다. 예를 들어, "notFoundImage"와 같이 이미지가 발견되지 않은 경우를 나타내는 이름으로 변경할 수 있습니다. - 필요한 최소한의 기능만 포함된 코드인 것 같으며, 그 외에 개선할 사항은 없어 보입니다. 그러나 코드 베이스의 전반적인 구조와 일관성을 확인하는 것이 좋습니다.
- 이미지 파일 이름과 실제 파일이 일치하는지 확인하십시오. 파일명의 철자나 대/소문자가 정확하지 않을 경우 이미지가 로드되지 않을 수 있습니다.
추가로, 코드 리뷰를 더 정확하게 할 수 있도록 전체 코드 및 관련 문맥을 제공해 주시면 감사하겠습니다.
@@ -143,5 +144,6 @@ enum StringLiterals { | |||
static let baseImageURL = "https://github.com/TeamDon-tBe/SERVER/assets/97835512/fb3ea04c-661e-4221-a837-854d66cdb77e" | |||
static let notificationImageURL = "https://github.com/TeamDon-tBe/SERVER/assets/128011308/327d416e-ef1f-4c10-961d-4d9b85632d87" | |||
static let warnUserGoogleFormURL = "https://forms.gle/FTgZKkajwtzFvAk99" | |||
static let errorMessage = "이런!\n현재 요청하신 페이지를 찾을 수 없어요!" | |||
} | |||
} |
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들과 경고 메시지 URL도 업데이트되었습니다. 이러한 변경은 이미지나 링크가 변경되었다면 필요한 수정으로 보입니다.
전반적으로, 주어진 코드 패치는 안전하고 예상되는 동작 작업을 위한 문자열 상수들을 추가하고 있습니다. 추가된 내용은 잘못된 점이 없으며, 안정성을 유지하기 위해 앱 컨텍스트와 일치하는지 확인하는 것이 좋습니다.
"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.
해당 코드는 Xcode에서 사용하는 이미지 정보를 정의하는 JSON 형식의 파일입니다.
이 코드 패치는 "img_404.png"라는 파일에 대해서 "1x", "2x", "3x"의 스케일을 가지는 이미지를 정의합니다.
버그 위험은 존재하지 않습니다.
개선 제안으로는, 아래와 같은 사항들이 있을 수 있습니다:
- 이미지의 확장자(.png)을 명시적으로 지정하고, 확장자가 올바른지 확인하는 것이 좋습니다.
- 파일 이름의 일관성을 유지하기 위해, "img_404"라는 부분을 변수로 추출해 사용할 수 있습니다.
- 구조화된 데이터를 사용하여, 이미지 관련 정보가 추가될 경우 손쉽게 확장할 수 있도록 할 수 있습니다.
let viewController = ErrorViewController() | ||
sceneDelegate.window?.rootViewController = UINavigationController(rootViewController: viewController) | ||
} | ||
} | ||
throw NetworkError.notFoundError | ||
case 500: | ||
throw NetworkError.internalServerError |
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.
이 코드 패치에 대한 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험과 개선 제안을 환영합니다:
- 92번 줄에서
donNetwork
함수 호출 시, 오타로 인해 "do"가 "don"으로 오타되었습니다. 이 부분을 수정해야 합니다. - 92번 줄 아래의 코드는 404 상태 코드일 때 실행됩니다.
UIApplication.shared.connectedScenes.first
에서await
키워드를 사용하고 있으므로 비동기적인 작업이 수행되고 있다고 가정합니다. 해당 코드 블록 내에서 UI 작업을 수행하고 있으며, 이는 메인 스레드에서 실행되어야 합니다. 따라서DispatchQueue.main.async
를 사용하여 UI 작업을 메인 큐에서 비동기적으로 실행하도록 변경하는 것이 좋습니다. 그럼으로써 UI 업데이트가 제대로 이루어지는 것을 보장할 수 있습니다. - 96번 줄과 98번 줄 사이에 들여쓰기가 잘못되어 있는 것 같습니다. 코드의 일관성을 위해 적절한 들여쓰기를 추가하는 것이 좋습니다.
그 외에는 코드에 큰 문제가 없어보입니다. 이 코드 리뷰를 참고하여 수정 및 개선을 진행하시면 됩니다.
} | ||
} | ||
} | ||
} |
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.
이 코드 패치는 ErrorViewController
클래스를 정의하는 것으로 보입니다. 주요 기능 및 UI 구성 요소가 제공되며, ErrorViewController
뷰 컨트롤러의 뷰 계층 구조, 레이아웃 및 이벤트 처리가 설정됩니다.
주요 개선 제안:
import UIKit
은 이미UIKit
프레임워크에 속하므로 중복으로 임포트하지 않아도 됩니다.SnapKit
라이브러리를 사용하여 UI 레이아웃 구성을 쉽게 할 수 있습니다.- 앱의 상태 바 높이 및 여백과 같은 값은 하드코딩 대신 값을 참조하여 사용하는 것이 좋습니다.
customButton
은 번역되어야 합니다.
버그/위험 사항:
- 예외 처리가 없으므로 문제가 발생할 경우 애플리케이션이 충돌할 수 있습니다.
- 코드 내에서 앱의 최상위 뷰 컨트롤러를 변경하는 메서드(
window?.rootViewController
)가 사용되었는데, 현재 시스템에 연결된 장면(delegate)이 있는지 확인하지 않습니다.
추가 제안:
- 에러 처리 로직을 추가하여 네트워크 오류 및 다른 예기치 않은 오류에 대한 조치를 취할 수 있습니다.
ErrorViewController
클래스의 테스트를 작성하여 코드가 예상한 대로 작동하는지 확인할 수 있습니다.
@@ -144,7 +144,7 @@ extension MyPageViewController { | |||
self.navigationController?.navigationBar.addSubviews(navigationBackButton) | |||
navigationBackButton.snp.makeConstraints { | |||
$0.centerY.equalToSuperview() | |||
$0.leading.equalToSuperview().inset(23.adjusted) | |||
$0.leading.equalToSuperview().inset(16.adjusted) | |||
} | |||
rootView.pageViewController.view.snp.makeConstraints { | |||
$0.top.equalTo(rootView.segmentedControl.snp.bottom).offset(2.adjusted) |
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.
해당 코드 패치를 간략히 검토해보겠습니다. 개선할 점과 버그 위험에 대한 의견을 드리겠습니다.
-
변경된 부분 중
$0.leading.equalToSuperview().inset(16.adjusted)
는 원래 값인23
에서16
으로 변경되었습니다. 이는 왼쪽 여백의 크기를 조정한 것입니다. -
수정된 코드에서
rootView.pageViewController.view.snp.makeConstraints
를 통해top
제약 조건이 설정되었는데,rootView.segmentedControl.snp.bottom
에서 2만큼 떨어진 위치로 지정되었습니다. -
해당 코드 패치의 문제점이나 버그 위험은 찾지 못했습니다. 그러나 프로젝트 전반적인 컨텍스트와 함께 실제 사용 시의 동작을 고려해야 합니다.
주의할 점: 위의 검토 내용은 기존 코드와의 차이점에 초점을 맞추어 일반적인 개선점을 제시한 것일 뿐, 애플리케이션에서의 동작 결함 등 개별적인 사항은 파악하지 못하였습니다. 전체 코드와 함께 테스트하여 문제를 발견하고 수정하는 것이 좋습니다.
@@ -51,7 +51,7 @@ final class MyPageProfileView: UIView { | |||
|
|||
let userIntroduction: UILabel = { | |||
let label = UILabel() | |||
label.setTextWithLineHeight(text: "안녕하세요히안우히히안녕하세요반가와요우히히히히안녕하세요히안우히히안녕하세요반가와요우히히히히", lineHeight: 20.adjusted, alignment: .center) | |||
label.setTextWithLineHeight(text: "", lineHeight: 20.adjusted, alignment: .center) | |||
label.textColor = .donGray7 | |||
label.textAlignment = .center | |||
label.font = .font(.caption2) |
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.
아래는 코드 패치이며, 해당 코드의 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안 사항은 환영합니다:
주목할 점:
-
라인 42에서
userNickname
레이블의 텍스트가 "안녕하세요반가와요우히히"로 고정되어 있습니다.- 개선 제안:
loadUserData()?.userNickname
으로 대체하여 사용자의 닉네임을 동적으로 설정하는 것이 좋습니다.
- 개선 제안:
-
라인 51에서
userIntroduction
레이블의 텍스트가 빈 문자열("")로 설정되어 있습니다.- 개선 제안: 필요한 텍스트를 적절하게 설정하여 사용자 소개를 나타내는 것이 좋습니다.
위의 주목할 점들을 고려하여 코드를 수정하시면 됩니다.
@objc | ||
func profileButton() { | ||
ProfileButtonAction() | ||
} | ||
} |
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.
다음은 코드 패치입니다. 해당 코드의 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 또는 개선 제안을 알려주세요.
ProfileButtonAction
변수가 추가되었습니다. 이 변수는 프로필 버튼 클릭 시 호출될 클로저를 저장하는 용도로 사용됩니다.image.isUserInteractionEnabled = true
구문이 추가되었습니다.image
뷰에 대한 사용자 상호 작용을 가능하게 합니다.profileImageView
에 탭 제스처 recognizer가 추가되었습니다.profileButton()
메서드가 탭 제스처를 처리하고,ProfileButtonAction()
클로저를 호출합니다. 이에 따라 프로필 이미지를 탭할 때ProfileButtonAction
에 지정된 동작이 실행됩니다.
개선할 점:
각 클로저 및 버튼의 액션 이름을 보다 명확하게 나타낼 수 있다면 가독성이 향상될 것입니다. 현재 KebabButtonAction
, LikeButtonAction
, TransparentButtonAction
, ProfileButtonAction
등은 개선의 여지가 있습니다. 더 의미있는 이름으로 변경해주는 것이 좋습니다.
cell.ProfileButtonAction = { | ||
self.memberId = self.viewModel.postReplyData[indexPath.row].memberId | ||
self.pushToMypage() | ||
} | ||
cell.nicknameLabel.text = viewModel.postReplyData[indexPath.row].memberNickname | ||
cell.transparentLabel.text = "투명도 \(viewModel.postReplyData[indexPath.row].memberGhost)%" | ||
cell.contentTextLabel.text = viewModel.postReplyData[indexPath.row].commentText |
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.
아래는 코드 패치에 대한 간단한 코드 리뷰입니다. 버그 위험과/또는 개선 제안이 있는 경우 알려주세요:
eraseToAnyPublisher
를 사용하여 publisher를 어떤 타입의 publisher로 지정하고 있습니다. 이 문제가 의도된 것인지 확인하세요.viewWillDisappear(_ animated: Bool)
메서드에서navigationController?.navigationBar
의 속성을 변경하고 있습니다. 이것이 원하는 동작인지 확인하세요.pushToMypage()
함수에서 새로운 뷰 컨트롤러를 생성하고 내비게이션 스택에 푸시하고 있습니다. 이 작업은 원하는 동작인지 확인하세요.ProfileButtonAction
클로저에서self.memberId
를 설정하고pushToMypage()
를 호출하고 있습니다. 클로저 동작이 원하는 대로 실행되는지 확인하세요.
이 외에는 보이는 문제가 없어 보입니다.
@@ -33,6 +33,7 @@ final class PostView: UIView { | |||
image.clipsToBounds = true | |||
image.layer.cornerRadius = 22.adjusted | |||
image.image = ImageLiterals.Common.imgProfile | |||
image.isUserInteractionEnabled = true | |||
return image | |||
}() | |||
|
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.
코드 패치에 대한 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 사항 및 개선 제안을 언급할 예정입니다:
- 해당 코드 패치에서는 이미지 뷰(
image
)의isUserInteractionEnabled
속성을true
로 설정하고 있습니다. 이로 인해 사용자의 상호 작용이 가능해집니다. 따라서, 해당 코드는 사용자가 이미지를 터치하거나 다른 상호 작용을 할 수 있도록 하는 용도로 사용되었다고 가정할 수 있습니다.
위의 사항을 감안하여 적절한 사용 방법인지 확인하십시오. 만약 이미지에 대한 상호 작용이 필요하지 않은 경우에는 isUserInteractionEnabled
속성을 false
로 설정하는 것이 좋을 수 있습니다.
그 외에는 코드 패치에서 별다른 버그 위험 사항은 보이지 않습니다.
$0.edges.equalToSuperview() | ||
} | ||
|
||
} | ||
} | ||
} | ||
|
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.
이 코드 패치에는 SnapKit 라이브러리를 추가하는 부분이 있습니다. 일부 개선 제안 및 버그 위험은 다음과 같습니다:
-
버그 위험:
getAPI()
메서드에서SnapKit
을 사용하기 전에UserDefaults
로부터 "memberGhost" 값을 가져오는 것이 좋습니다. 현재 코드에서는getAPI()
가 호출되기 전에 "memberGhost" 값을 검사하므로,SnapKit
을 사용하기 전에 체크하도록 변경해야 합니다. -
향상 제안:
banView
를window
에 추가하는 부분에서addSubview
대신insertSubview(_:at:)
를 사용하는 것이 더 명확할 수 있습니다. 예를 들어,window.insertSubview(banView, at: 0)
와 같이 사용할 수 있습니다. -
개선 제안:
promiseButtonTapped()
메서드에서 뒤로가기 동작을 처리하고,banView
를 제거한 후에도 즉시popViewController
를 호출하는 대신, 애니메이션의 완료 시점에서popViewController
를 호출하는 것이 더 안전합니다.promiseButtonTapped()
메서드 내에서 다음과 같이 변경하여 수정할 수 있습니다:
@objc
private func promiseButtonTapped() {
UIView.animate(withDuration: 0.3, animations: {
self.banView.alpha = 0
}) { _ in
self.banView.removeFromSuperview()
self.navigationController?.popViewController(animated: true)
}
}
위의 개선 제안을 참고하여 코드를 수정할 수 있습니다.
if UserDefaults.standard.integer(forKey: "memberGhost") > -85 { | ||
contentTextView.becomeFirstResponder() | ||
NotificationCenter.default.addObserver(self, selector: #selector(keyboardWillShow(_:)), name: UIResponder.keyboardWillShowNotification, object: nil) | ||
} | ||
// 햅틱 피드백 생성 | ||
impactFeedbackGenerator.prepare() | ||
} |
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.
이 코드 패치에 대한 간략한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안 사항을 환영합니다.
주의: 본 답변은 2021년 9월 1일 이전의 지식을 바탕으로 하며, 과거 정보일 수 있으므로 실제 상황에 맞추어 검토해야 합니다.
개선 제안:
- 코드가 잘 작성되어 보입니다. 현재 로그인한 사용자의
memberGhost
값을 확인하여 특정 조건에 따라contentTextView.becomeFirstResponder()
를 호출하는 부분을 추가하도록 수정되었습니다. NotificationCenter.default.addObserver(_:selector:name:object:)
의addObserver
를 호출하기 전에 'keyboardWillShowNotification' 에 대한 옵저버를 등록하도록 변경되었습니다.limitedCircleProgressBar.alpha = 0
문장은 삭제되었습니다. 해당 라인이 다른 곳에서 사용되었다면, 관련된 코드를 다시 점검해야 할 수도 있습니다.impactFeedbackGenerator
를 생성하고 있는 부분은 유지되었습니다.
버그 위험:
- 현재의 코드 패치에서 메모리 누수가 발생하지 않는지 확인해야 합니다.
addObserver(_:selector:name:object:)
를 호출한 후 우리는 뷰 컨트롤러 인스턴스(현재 클래스의 인스턴스)를 가리키는 익명 클로저를 매개변수로 사용하지 않는 것이 좋습니다. 이 경우, 메모리 누수 문제가 발생하고 있을 수 있습니다. - 또한,
removeObserver(_:name:object:)
를 호출하여 옵저버를 제대로 해제해야 합니다. 해당 클래스 인스턴스가 해제되거나 뷰 컨트롤러의 생명 주기에서 적절한 시점에 옵저버를 제거하는 것이 중요합니다.
위의 개선 제안과 함께 버그 위험 사항을 고려하여 코드를 검토하시기 바랍니다.
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.
P3
404페이지 너무 이쁘고 깔끔하네요~~ 프로필 클릭 시 유저페이지 이동하는것도 나이스입니다~~
@@ -136,6 +136,7 @@ extension MyPageViewModel { | |||
accessToken: accessToken, | |||
body: EmptyBody(), | |||
pathVariables: ["":""]) | |||
UserDefaults.standard.set(result?.data?.memberGhost ?? 0, forKey: "memberGhost") |
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.
P3
투명도 값 저장해놓으면 나중에 사용할때 편하겠네요!!! 감사합니다ㅏ
if UserDefaults.standard.integer(forKey: "memberGhost") > -85 { | ||
contentTextView.becomeFirstResponder() | ||
NotificationCenter.default.addObserver(self, selector: #selector(keyboardWillShow(_:)), name: UIResponder.keyboardWillShowNotification, object: nil) | ||
} |
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.
P3
글쓰기 제한 분기처리 고생하셨습니다!!
👻 PULL REQUEST
💻 작업한 내용
💡 참고사항
📸 스크린샷
구현
상세뷰에서 프로필
클릭시
유저페이지로 이동
분기처리
📟 관련 이슈