Skip to content
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 [#151] 페이지네이션 구현 #158

Merged
merged 16 commits into from
Mar 1, 2024
Merged

Conversation

yeonsu0-0
Copy link
Collaborator

👻 PULL REQUEST

💻 작업한 내용

페이지네이션 구현 완료했습니다.

💡 참고사항

다음과 같이 밑으로 스크롤 시 작동되는 함수를 이용하여 cursor 값을 변경시켜 데이터를 불러오도록 했습니다.

func scrollViewDidEndDragging(_ scrollView: UIScrollView, willDecelerate decelerate: Bool) {
if scrollView == homeCollectionView {
if (scrollView.contentOffset.y + scrollView.frame.size.height) >= (scrollView.contentSize.height) {
let lastContentId = homeViewModel.postDatas.last?.contentId ?? -1
homeViewModel.cursor = lastContentId
bindViewModel()
DispatchQueue.main.async {
self.homeCollectionView.reloadData()
}
}
}
}

📸 스크린샷

기능 스크린샷
GIF

📟 관련 이슈

Copy link
Member

@boogios boogios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3
페이지네이션 구현 모든 뷰에 적용시키느라 고생많으셨습니다!!!!!
특히 마이페이지ㅠㅠ 복잡한 구조 이해하시느라 고생하셨습니당!!!

self.homeCollectionView.reloadData()
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3
페이지네이션 함수 너무 좋네용!!

@@ -104,6 +106,11 @@ extension HomeViewModel {
tempArrayData.append(content)
}
self.postData = tempArrayData
if cursor == -1 {
postDatas = data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3
처음일 때, 분기처리 좋습니당~

destinationViewController.contentId = homeViewModel.postData[indexPath.row].contentId
destinationViewController.memberId = homeViewModel.postData[indexPath.row].memberId
destinationViewController.contentId = homeViewModel.postDatas[indexPath.row].contentId
destinationViewController.memberId = homeViewModel.postDatas[indexPath.row].memberId
self.navigationController?.pushViewController(destinationViewController, animated: true)
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰:

  1. postDatapostDatas로 변경한 내용에 따른 수정은 일관성을 유지하는 좋은 변경입니다.
  2. scrollViewDidEndDragging 함수: 코드는 맞지만, 사용자 경험 측면에서 추가 스크롤 시 너무 많은 항목 로드될 수 있으니 더 효율적인 로딩 방식 고려 필요.
  3. collectionView(_: cellForItemAt:) 함수: 클로저 처리 등으로 인한 메모리 관리 측면에서 주의 필요.
  4. ProfileButtonAction 클로저: 잠재적인 강한 순환 참조 문제가 발생할 수 있습니다. [weak self] 사용을 고려해보세요.
  5. 코드 중복: cell.likeButton.setImage 부분을 줄일 수 있으며, DRY 원칙에 따라 개선이 필요합니다.
  6. didSelectItemAt 함수: 새 PostDetailViewController를 생성하는 방법 대신, 기존 객체를 재활용하여 생성 시간을 줄이는 것이 바람직합니다.

개선 제안:

  1. 무한 스크롤 방식 개선 / 로딩 성능 향상 고려
  2. 메모리 누수 예방을 위한 클로저 관리 개선
  3. 순환 참조 및 메모리 누수 방지를 위해 [weak self] 사용 늘리기
  4. 코드 중복 최소화 및 재사용성 높이기
  5. UI 업데이트 관련 작업은 메인 스레드에서 관리되도록 보장

좋은 점:

  1. 데이터 모델 명명 일관성
  2. 확장 가능한 코드 구조
  3. 비동기 UI 업데이트를 위한 DispatchQueue 활용

추가로, 코드의 유지보수성과 가독성을 높이기 위해 주석 등을 상세히 작성하는 것이 유용할 수 있습니다.

postDatas = data
} else {
postDatas.append(contentsOf: postData)
}
}
return result
} catch {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 하면서 발견된 사항은 다음과 같습니다:

  1. postDatas 배열은 초기화되지 않았고, 코드에서 중복으로 데이터가 추가되는 부분이 있습니다. 초기화와 신중한 데이터 추가 및 처리가 필요합니다.
  2. baseURL 값을 "/content/all"에서 "/contents"로 변경했지만, 이 변경에 대한 근거가 설명되어 있지 않습니다. 변경 시의 영향을 고려할 필요가 있습니다.
  3. cursor 변수는 -1로 초기화되었지만, 어디서 사용되는지 명확하게 나타나지 않습니다. 해당 변수를 정확하게 사용 목적에 맞게 이름설정하고 주석을 달아서 코드 이해를 돕는 것이 좋습니다.

개선 제안:

  1. postDatas 배열을 초기화해주도록 수정하고, 데이터 추가하는 로직을 명확하게하여 중복 추가되지 않도록 조치하세요.
  2. baseURL의 변경 이유를 주석으로 설명하거나 필요 시 팀원들과 의논하여 결정 및 변경 이유를 문서화하세요.
  3. cursor 변수의 역할을 명확히 하고 적절한 변수명을 선택하며, 주석을 추가하여 코드 가독성을 높이세요.

}
}
}

func collectionView(_ collectionView: UICollectionView, viewForSupplementaryElementOfKind kind: String, at indexPath: IndexPath) -> UICollectionReusableView {
guard let footer = homeCollectionView.dequeueReusableSupplementaryView(ofKind: UICollectionView.elementKindSectionFooter, withReuseIdentifier: "HomeCollectionFooterView", for: indexPath) as? HomeCollectionFooterView else { return UICollectionReusableView() }
return footer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치의 간단한 코드 리뷰입니다.

  • commentDatacommentDatas로 변경하면서 데이터 구조를 수정했습니다. 모든 인스턴스 및 참조도 수정되어야 합니다.
  • MyPageViewModel을 초기화하여 myPageViewModel 속성을 추가합니다.
  • scrollDidEndDragging 함수가 페이지네이션 기능을 구현하는 데 도움을 줄 수 있습니다. 그러나 일부 처리는 UI 스레드에서 발생해야 합니다.
  • 프로필 및 코멘트 정보를 셀에 제대로 반영하고 있는지 확인 필요합니다.
  • warningBottomsheetdeleteBottomsheet에 관련된 동작 및 이벤트 처리가 정확한지 다시 검토해야 합니다.

더 많은 상세 내용을 파악하려면 전체 애플리케이션 흐름과 목적을 고려해야 합니다. 에러 처리와 보안 상태에 대한 심도 있는 테스트를 수행하는 것이 중요합니다.

}
}
}

func collectionView(_ collectionView: UICollectionView, viewForSupplementaryElementOfKind kind: String, at indexPath: IndexPath) -> UICollectionReusableView {
guard let footer = homeCollectionView.dequeueReusableSupplementaryView(ofKind: UICollectionView.elementKindSectionFooter, withReuseIdentifier: "HomeCollectionFooterView", for: indexPath) as? HomeCollectionFooterView else { return UICollectionReusableView() }
return footer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰 내용은 다음과 같습니다:

  1. contentDatacontentDatas로 변경했지만, 기존에 주석 처리된 코드인 var contentData = MyPageViewModel(networkProvider: NetworkService()).myPageContentDatas가 아직 사용되고 있습니다. 이 부분의 주석 처리나 해당 변수 사용을 검토해야 할 것입니다.

  2. UICollectionViewDataSourceUICollectionViewDelegate에서 인덱스 변화(indexPath.row)를 사용할 때, contentDatas 배열의 범위를 넘어갈 수 있는 잠재적 위험이 있습니다.(예: self.contentDatas[indexPath.row].memberId) 범위 체크를 추가하거나 안전한 방어적 코드를 고려해야 합니다.

  3. scrollViewDidEndDragging 함수에서 post 메서드 호출 전에 self.homeCollectionView.reloadData()가 있어 UI 갱신 비용이 늘어날 수 있습니다. 가능하면 데이터 업데이트 후 한꺼번에 UI를 갱신하는 방안을 고려해보세요.

  4. 일부 주석이 영문으로 되어 있으며, 코드 작성자 및 유지 보수 개발자들을 위해서 주석을 한 언어로 통일하는 것이 좋습니다.

  5. 클래스 및 메서드의 네이밍은 명확하지만 가독성을 높이는 데 도움이 될 수 있습니다. 더 구체적이고 직관적인 이름으로 변경하는 것을 고려해보세요.

  6. 앱 아키텍처 및 디자인 패턴에 대한 적절한 설계 고려 필요성을 확인하세요. 현재 코드 스니펫에서는 MVC(Model View Controller)를 사용하는 것으로 보입니다.

  7. 코드의 재사용 가능성 및 확장성을 고려하여 중복 코드나 유사한 패턴을 함수나 메서드로 추출하여 관리하는 것이 좋을 수 있습니다.

@boogios boogios merged commit 0727d6e into develop Mar 1, 2024
1 check passed
@boogios boogios deleted the feat/#151-pagination branch March 1, 2024 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 페이지네이션 구현
2 participants