Feat [#58] 홈 + 게시글 뷰 액션 추가 #66
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -951,6 +966,7 @@ | |||
| 2A28453E2B531DDE0023F9B5 /* NetworkService.swift in Sources */, | |||
| 2AC9FB1B2B4DE77400D31071 /* AgreementListCustomView.swift in Sources */, | |||
| 2F17418A2B500CC20089FC4D /* HomeBottomsheetView.swift in Sources */, | |||
| 2FBBADF82B53EF63002D6286 /* PostPopupView.swift in Sources */, | |||
| 2FB64FF02B5310DD0082A414 /* WriteReplyViewController.swift in Sources */, | |||
| 3C01692A2B4DC82D0075334B /* DontBePopupView.swift in Sources */, | |||
| 2A2671FF2B4C3AF0009D214F /* Publisher+UIControl.swift in Sources */, | |||
There was a problem hiding this comment.
위의 코드는 파일과 디렉토리 구조를 수정하는 것으로 보입니다. 주요 변경 내용은 다음과 같습니다:
- [추가]
TransparentPopupViewController.swift,CancelReplyPopupViewController.swift,PostPopupView.swift,DeletePopupViewController.swift파일이 프로젝트에 추가되었습니다. - 해당 파일들과 관련된 Build Files 및 File References도 추가되었습니다.
ViewControllers및 다른 디렉토리에 새로운 파일이 추가되었습니다.- 소스 및 리소스 파일의 빌드 단계에서 해당 파일들이 정상적으로 처리되도록 업데이트되었습니다.
코드 조각에 대한 완전한 검토는 파일 및 프로젝트의 전반적인 컨텍스트에 의해 좌우될 수 있습니다. 코드 문제나 버그 위험을 파악하기 위해서는 추가적인 정보가 필요합니다. 그러나 일단 작업이 수행된 내용만으로는 큰 문제를 발견하기는 어렵습니다.
| static let popupReplyContentLabel = "작성 중인 글을 삭제하시겠어요?" | ||
| static let popupReplyCancelButtonTitle = "취소" | ||
| static let popupReplyConfirmButtonTitle = "삭제" | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
enum StringLiterals {
static let transparentPopupContentLabel = "지금 누르신 투명도 기능이 Don’t be를 더 온화한 커뮤니티로 만들기 위한 일이겠죠?"
static let transparentPopupLefteftButtonTitle = "조금 더 고민하기"
static let transparentPopupRightButtonTitle = "네, 맞아요"
static let deletePopupTitleLabel = "게시글을 삭제하시겠어요?"
static let deletePopupContentLabel = "삭제한 게시글은 영구히 사라져요."
static let deletePopupLefteftButtonTitle = "취소"
static let deletePopupRightButtonTitle = "삭제"
}
enum Post {
static let navigationTitleLabel = "게시글"
static let textFieldLabel = "님에게 답글 남기기"
static let popupReplyContentLabel = "작성 중인 글을 삭제하시겠어요?"
static let popupReplyCancelButtonTitle = "취소"
static let popupReplyConfirmButtonTitle = "삭제"
}
이 코드 패치의 리뷰를 간단히 해 드리겠습니다.
버그 위험 및 개선 제안:
- 코드에서 명시적으로 오류나 버그는 보이지 않습니다.
- 그러나 문자열 리터럴에 대해 일관된 네이밍 규칙을 사용하는 것이 좋습니다. 일부 변수 이름에 "Lefteft"와 "CancelButton", "ConfirmButton"와 같이 오타가 있는 것으로 보입니다. 이러한 오타를 수정하는 것이 권장됩니다.
- 문자열 내용은 언어와 문화에 따라 다르므로, 번역 및 표현에 대해 확인하고 제안을 할 수 있는 프로젝트의 사용자 또는 로컬라이제이션 전문가에게 의견을 요청하는 것이 좋습니다.
| @@ -126,7 +130,7 @@ extension DontBeBottomSheetView { | |||
| $0.top.equalTo(19.adjusted) | |||
| } | |||
|
|
|||
| singleButton.snp.makeConstraints { | |||
| deleteButton.snp.makeConstraints { | |||
| $0.centerX.equalToSuperview() | |||
| $0.width.equalTo(344.adjusted) | |||
| $0.height.equalTo(60.adjusted) | |||
There was a problem hiding this comment.
코드 리뷰:
-
dimView와bottomsheetView변수를 private으로 선언하는 대신, 로컬 변수로 선언하고 초기화하기 때문에 수정이 필요하지 않습니다. -
singleButton을deleteButton으로 이름 변경하였으며,warnButton이라는 새로운 버튼을 추가하였습니다. -
profileEditButton의 이미지 설정에서 잘못된 이미지(ImageLiterals.Posting.btnDelete)를 사용하고 있기 때문에 수정이 필요합니다. -
init(singleButtonImage:)메서드에서 더 이상singleButton의 이미지를 설정하고 있지 않기 때문에 해당 부분이 삭제되어야 합니다. -
setLayout()메서드에서singleButton대신deleteButton의 제약 조건을 설정해야 합니다. -
코드의 다른 부분은 상당히 깔끔해 보입니다.
| // ✅ 투명도 주기 버튼 클릭 시 액션 추가 | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
이 코드 패치는 DeletePopupViewController.swift 파일에 대한 것으로 보입니다. 이곳에서는 제목과 내용이 있는 팝업을 표시하고, 팝업의 왼쪽 및 오른쪽 버튼에 대한 동작을 처리하는 DeletePopupViewController 클래스가 정의되어 있습니다.
해당 코드는 큰 문제 없이 작성되어 보이며, 주요한 버그 위험 요소는 보이지 않습니다. 다만, 아직 구현되지 않은 getAPI() 메서드가 존재하는데, 해당 함수가 어떤 네트워크 작업을 수행할지 구체적으로 추가하는 것이 좋을 것 같습니다.
또한, setUI(), setHierarchy(), setLayout(), setDelegate() 메서드는 현재 비어 있어서 실제 로직을 추가해야 합니다. 빈 상태로 남겨둔 것인지 구현이 필요한 로직이 아직 정해지지 않은 것인지 파악해야 합니다.
마지막으로, confirmButtonTapped() 메서드에서 "투명도 주기 버튼 클릭 시 액션 추가"라는 주석이 있으나, 실제로 어떤 동작을 추가해야 하는지 명확하게 표현되지 않았습니다. 이 내용을 명확하게 구체화하여 해당 동작을 구현하는 방법에 대한 주석을 추가하는 것이 좋습니다.
| transparentButtonPopupView.alpha = 0 | ||
| // ✅ 투명도 주기 버튼 클릭 시 액션 추가 | ||
| } | ||
| } |
There was a problem hiding this comment.
해당 코드의 주요 내용은 다음과 같습니다.
deleteBottomsheet변수가 추가되었으며,deletePost메서드에서 해당 뷰를 제거하고 삭제 팝업 뷰(deletePostPopupVC)를 표시합니다.transparentPopupVC및deletePostPopupVC도 추가되었습니다.setAddTarget메서드에서deleteBottomsheet의deleteButton에 대한 타겟 및 액션을 설정합니다.popView메서드는deleteBottomsheet를 페이드 아웃하고 숨깁니다.presentView메서드는deletePostPopupVC를 호출하여 표시합니다.dismissViewController메서드는 뷰 컨트롤러를 해제합니다.setDelegate메서드에서homeCollectionView의 데이터 소스와 델리게이트를 설정합니다.refreshPost메서드와finishedRefreshing메서드는 새로 고침 기능과 관련된 로직입니다.showToast메서드는 토스트를 표시하는 로직을 처리합니다.TransparentButtonAction클로저에서는transparentPopupVC를 호출하여 표시합니다.- 마지막으로,
DontBePopupDelegate프로토콜을 사용하여 팝업 버튼 동작을 처리하도록합니다.
개선할 점:
- 변수 및 메서드명이 명확하게 표현되도록 한다.
- 중복된 코드를 최소화하는 방향으로 리팩터링한다.
UIRefreshControl을 구현할 때 알맞은 iOS 지침을 따르도록 한다.
위의 내용을 참고하여 코드를 개선하고 가독성을 높일 수 있습니다.
| self.dismiss(animated: false) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
이 코드 패치는 상대적으로 간단한 기능을 구현하고 있습니다. 다음은 코드 리뷰입니다:
-
cancelPopupView,myView,writeReplyVC와 같은 변수명은 읽기 어려울 수 있습니다. 좀 더 명확하고 직관적인 변수명을 사용하는 것이 좋습니다. -
getAPI()메서드는 현재 아무런 동작도 수행하지 않습니다. 이러한 경우 주석을 사용하여 왜 해당 메서드가 존재하는지를 설명하는 것이 좋습니다. -
loadView()함수에서view속성을 설정하고 있는데,myView를 만들어서view에 할당하는 대신cancelPopupView를 직접view에 추가하는 것이 효율적일 수 있습니다. -
Delegate관련 메서드(setDelegate())는 해당 프로토콜에 대한 구현이 없으므로 제거해도 됩니다. -
cancleButtonTapped()과confirmButtonTapped()에서self.dismiss(animated: false)를 호출하기 전에 경고 메시지를 표시하는 것이 좋습니다. 사용자가 팝업을 닫아도 되는지 확인하는 것이 좋습니다. -
CancelReplyPopupViewController.popViewController는 스태틱 변수로 정의되어 있는데, 변수명은 전역상수나 열거형 형태로 작성하는 것이 좋습니다. 변수를 소문자로 시작하는popViewController로 변경하는 것이 좋습니다. -
프로토콜 지향 프로그래밍을 적용하여,
private메서드의 이름 앞에_를 붙여 구분하는 것이 좋습니다. -
코드에 대한 주석은 아직 제공되지 않아 어떤 기능을 하고 있는지 파악하기 어렵습니다. 주석을 추가하면 이해하기 쉬운 코드가 될 수 있습니다.
추가적으로 리팩토링과 개선에 대한 의견은 소유자에게 게시된 코드와 설명에 따라 다를 수 있으니 참고 바랍니다.
| // present | ||
| self.present(self.transparentPopupVC, animated: false, completion: nil) | ||
| } | ||
|
|
||
| return cell | ||
| } | ||
|
|
There was a problem hiding this comment.
다음은 코드 패치에 대한 간단한 코드 리뷰입니다. 버그 가능성 및 개선 제안 사항 환영합니다.
deleteBottomsheet와transparentPopupVC를 생성할 때 인스턴스화되지 않은 것으로 보입니다. 이러한 객체를 인스턴스화해야 합니다.uploadToastView는 nil 값인 상태에서 뷰에 추가되고 있습니다. UIViewContorller의viewDidLoad()메서드 내에서 인스턴스화 후 추가하는 것이 좋습니다.showToast(_:)메서드 내에서 애니메이션 블록의 실행이 비동기적으로 이루어지므로, 루프와 딜레이가 필요하지 않을 것입니다. UIView.animate를 통해 바로 애니메이션을 설정하는 방법을 고려해 볼 수 있습니다.setNotification()메서드에서addObserver(_:selector:name:object:)호출 시에 문자열 중복을 피하기 위해#selector(...)를 사용하는 것이 좋습니다.popView()및presentView()내에서 UI 변경이 오류 없이 이루어지는지 확인해볼 필요가 있습니다. 제공된 코드만 보면 문제 없어 보입니다.dismissViewController()메서드는 어디서도 호출되지 않아 사용 여부를 확인해야 합니다.
일반적으로 제시된 코드에서 심각한 버그나 문제를 발견하지 못했습니다. 하지만 이외에도 유지 관리, 가독성 및 확장성을 개선하기 위해 다양한 방법으로 코드를 개선할 수 있습니다.
| self.dismiss(animated: false) | ||
| // ✅ 투명도 주기 버튼 클릭 시 액션 추가 | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드는 리뷰가 짧아서 전체 내용을 파악하기 어렵습니다. 주로 누락된 부분은 버그나 개선 제안에 대한 구체적인 내용입니다. 그러나 주어진 범위에서 다음과 같은 점들을 간단히 확인할 수 있습니다:
- 이 파일은 트랜스패런트 팝업 뷰 컨트롤러를 정의합니다.
TransparentPopupViewController클래스는UIViewController를 상속받으며,DontBePopupDelegate프로토콜도 구현합니다.TransparentPopupViewController클래스 내부에TransparentButtonPopupView와PostPopupView를 선언하고 초기화하는 코드가 있습니다.loadView()와viewDidLoad()메서드 재정의에서 데이터 요청(getAPI()), UI 설정(setUI()), 뷰 계층 구조 설정 (setHierarchy()), 레이아웃 설정 (setLayout()), 델리게이트 설정 (setDelegate())등이 실행됩니다.DontBePopupDelegate프로토콜을 구현한cancleButtonTapped()와confirmButtonTapped()메서드를 가지고 있으며, 버튼이 탭되었을 때 해당 팝업 뷰 컨트롤러를 닫습니다.
더 자세한 내용을 파악하기 위해서는 프로젝트의 다른 파일이나 클래스에 대한 정보가 필요합니다. 코드를 파악하는 데 있어서 특히 중요한 부분은 DontBePopupView, ImageLiterals, StringLiterals 등 각자 정의된 다른 컴포넌트들입니다. 이러한 부분에서 발생할 수 있는 문제 또는 개선점을 확인하려면 해당 파일의 내용을 참조해야 합니다.
|
|
||
| public func dismissView() { | ||
| self.dismiss(animated: true) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Network |
There was a problem hiding this comment.
이 코드 패치는 몇 가지 개선 사항과 버그 위험을 가지고 있습니다.
-
showUploadToastNotification은WriteReplyViewController의 내부 정적 속성처럼 보이지만, 코드 상단에 전역적으로 선언되어 있습니다. 그래서 다른 클래스에서 액세스하여 오류를 발생시킬 수 있습니다.WriteReplyViewController내부로 이동하는 것이 좋습니다. -
myView변수 이름은writeView로 변경하는 것이 좋습니다. 명확하고 의미 있는 이름을 사용하는 것이 코드의 가독성을 향상시킵니다. -
cancelReplyPopupVC를setNavigationBarButtonItem()에서 처음 생성하는 대신, 프로퍼티로 선언하고loadView()에서 초기화하는 것이 더 적합합니다. -
setAddTarget()메서드를 추가하여postButton에 대한 타겟 및 액션을 설정하는 것은 좋은 방법입니다. 그러나postButtonTapped()메서드가 잘못된 위치에 있으며, UIPresentationController를 삭제하지 않고 데이터를 보내고 있으므로 수정이 필요합니다.sendData()를 호출하기 전에popupNavigation()을 호출하여 화면을 닫아야 합니다. -
popupNavigation()메서드에서self.dismiss(animated: true)를 호출하고 있지만, 해당 뷰 컨트롤러가 모달 형태로 표시되므로self대신에presentingViewController?.dismiss(animated: true)를 사용하는 것이 좋습니다. -
dismissView()메서드는 현재 사용되지 않으며, 해당 메서드를 제거하는 것이 좋습니다. -
네트워크 관련 코드는 현재 주어지지 않아 리뷰할 수 없습니다.
개선 사항을 요약하면 다음과 같습니다:
showUploadToastNotification의 범위 제한- 변수 이름 변경 (
myView->writeView) cancelReplyPopupVC프로퍼티로 이동postButtonTapped()메서드 수정 (데이터 전송 후 팝업 닫기)popupNavigation()수정 (self.dismiss(animated: true)->presentingViewController?.dismiss(animated: true))- 사용되지 않는
dismissView()제거
| private func setDataBind() { | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
이 코드 패치는 PostPopupView라는 클래스를 정의하고 있습니다. 이 클래스는 UIView를 상속하며, SnapKit 라이브러리를 사용합니다.
코드 리뷰를 하면서 지적할 점이나 개선 사항은 다음과 같습니다:
Properties,UI Components,Life Cycles등 섹션 주석에 해당하는 내용을 구현해주어야 합니다.setUI(),setHierarchy(),setLayout(),setAddTarget(),setRegisterCell(),setDataBind()메소드는 현재는 비어있어서 어떤 동작을 하는지 알 수 없습니다. 이들 메소드에 필요한 로직을 구현해야 합니다.setDataBind()메소드가 비어있는데, 이 메소드는 어떤 데이터와 뷰를 바인딩하는 역할을 할 것으로 보입니다. 따라서 이 메소드를 활용하여 데이터와 뷰를 연결하는 코드를 구현해야 합니다.
또한, 위 코드에서는 기타 구체적인 내용(예: setUI(), setHierarchy() 등)이 구현되지 않았으므로 추가적인 정보가 필요합니다. 올바른 리뷰와 추가 피드백을 위해서는 코드의 전체적인 목적과 예상된 동작에 대한 정보가 필요합니다.
|
|
||
| // MARK: - UI Components | ||
|
|
||
| private let deletePostPopupView = DontBePopupView(popupTitle: StringLiterals.Home.deletePopupTitleLabel, |
| TransparentButtonAction() | ||
| isLiked.toggle() | ||
| likeButton.setImage(isLiked ? ImageLiterals.Posting.btnFavoriteActive : ImageLiterals.Posting.btnFavoriteInActive, for: .normal) | ||
|
|
||
| } | ||
|
|
||
| private func setRegisterCell() { |
There was a problem hiding this comment.
코드 리뷰는 다음과 같습니다:
KebabButtonAction,LikeButtonAction,TransparentButtonAction등의 액션 클로저 변수들이 삭제되고,deleteBottomsheet라는 새로운 속성이 추가되었습니다.ghostButton속성은private키워드가 제거되었습니다.
개선 사항:
@objc어노테이션이 붙은 메소드들은 외부에서 호출되지 않는 것으로 보입니다. 이런 경우에는private접근 제한자를 사용하는 것이 좋습니다.likeToggleButton()메소드 내에서 좋아요 버튼 이미지를 설정할 때, 상태를 변경하고 적절한 이미지를 설정하는 코드가 포함되어야 합니다. 현재로서는 버튼이 클릭될 때 단지 이미지만 변경되고 상태가 업데이트되지 않는 것으로 보입니다.
버그나 위험 요소는 확인되지 않았으며 주요 변경 사항은 속성과 메소드 이름, 접근 제한자의 수정입니다.
| @@ -79,7 +79,7 @@ final class WriteReplyEditorView: UIView { | |||
| return circle | |||
| }() | |||
|
|
|||
| private let postButton: UIButton = { | |||
| let postButton: UIButton = { | |||
| let button = UIButton() | |||
| button.setTitle(StringLiterals.Write.writePostButtonTitle, for: .normal) | |||
| button.setTitleColor(.donGray9, for: .normal) | |||
There was a problem hiding this comment.
이 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다.
버그나 개선 사항은 다음과 같습니다:
-
버그:
postButton속성이private에서 제거되었습니다. 이로 인해 해당 속성에 접근할 수 있는 범위가 확장됩니다. 이는 의도치 않은 사용을 허용할 수 있으므로 확인해야 합니다. -
개선점:
postButton변수가 클래스의 외부에서 사용되지 않는다면,let으로 선언하는 대신private let postButton으로 다시 변경하여 캡슐화 원칙을 따르는 것이 좋습니다.
이외에 다른 중요한 문제나 개선점은 보이지 않습니다.
👻 PULL REQUEST
💻 작업한 내용
💡 참고사항
📸 스크린샷
📟 관련 이슈