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.

Sign up for GitHub

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

Jump to bottom

Fix [#162] 로딩뷰 수정 #165

Merged
merged 3 commits into from
Mar 15, 2024
Merged

Fix [#162] 로딩뷰 수정 #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c53d823
[Delete] #162 - 기존 HomeVC에 구현해둔 로딩뷰 제거
yeonsu0-0 Mar 12, 2024
c1475db
[Feat] #162 - DontBeTabBarController에 로딩뷰 추가
yeonsu0-0 Mar 12, 2024
f3ad506
[Fix] #162 - 로딩뷰 나타나는 길이 수정
yeonsu0-0 Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions DontBe-iOS/DontBe-iOS/Presentation/Home/ViewControllers/HomeViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,6 @@ final class HomeViewController: UIViewController {
self.navigationController?.navigationBar.isHidden = true
self.tabBarController?.tabBar.isHidden = false

if !hasAppearedBefore {
hasAppearedBefore = true
} else {
showLoadingView()
}

bindViewModel()
setNotification()
}
Expand Down Expand Up @@ -129,18 +123,6 @@ extension HomeViewController {
deletePostPopupVC.modalPresentationStyle = .overFullScreen
}

private func showLoadingView() {
let loadingView = DontBeLoadingView()
print("로딩 시작!")
loadingView.show()

DispatchQueue.main.asyncAfter(deadline: .now() + 3) {
loadingView.hide {
print("로딩 끝!")
}
}
}

private func setHierarchy() {
view.addSubviews(homeCollectionView)
}
Copy link

Choose a reason for hiding this comment

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

코드 리뷰 요약:

  1. hasAppearedBefore 변수가 사용되지 않고 있으므로, 관련 코드를 제거해도 될 것으로 보입니다.
  2. showLoadingView() 메서드 역시 사용되지 않고 있으므로, 삭제하는 것이 좋을 것 같습니다.
  3. 주석이 없어서 코드의 의도를 명확히 이해하기 어려울 수 있습니다. 각 기능에 대한 주석 추가는 도움이 될 수 있습니다.
  4. 함수와 변수 이름에서 더 명확한 의미 전달을 위해 네이밍 규칙을 통일하고, 의미있는 이름을 선택할 수 있습니다.
  5. 코딩 스타일 일관성을 유지하는 것이 중요합니다. 들여쓰기 및 스타일을 일관성 있게 유지해야 합니다.

개선 사항:

  1. 사용되지 않는 변수 및 메서드 제거
  2. 적절한 주석 추가
  3. 네이밍 규칙 준수
  4. 코딩 스타일 일관성 유지

위의 내용을 참고하여 코드를 개선하시기 바랍니다.

Expand Down
20 changes: 20 additions & 0 deletions DontBe-iOS/DontBe-iOS/Presentation/TabBar/DontBeTabBarController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,14 @@ extension DontBeTabBarController: UITabBarControllerDelegate {
self.tabBar.items?[2].image = ImageLiterals.TabBar.icnNotificationRead
}

if beforeIndex == 2 && self.selectedIndex == 0 {
showLoadingView()
}

if beforeIndex == 3 && self.selectedIndex == 0 {
showLoadingView()
}

if let selectedViewController = tabBarController.selectedViewController {
applyFontColorAttributes(to: selectedViewController.tabBarItem, isSelected: true)
}
Expand Down Expand Up @@ -200,3 +208,15 @@ extension DontBeTabBarController: UITabBarControllerDelegate {
return true
}
}

extension DontBeTabBarController {
private func showLoadingView() {
let loadingView = DontBeLoadingView()
loadingView.show()

DispatchQueue.main.asyncAfter(deadline: .now() + 1.4) {
loadingView.hide {
}
}
}
}
Copy link

Choose a reason for hiding this comment

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

주요 코드 리뷰:

  1. showLoadingView 메서드에서 로딩 뷰를 보여주고 숨기는 로직이 있습니다. 그러나 애니메이션 블록 내부에 아무 작업도 수행하지 않는 부분이 있습니다. 이렇게 하는 대신 완료 핸들러 클로저에 필요한 작업을 추가하는 것이 좋습니다.

  2. beforeIndex == 2 및 beforeIndex == 3일 때 selectedIndex == 0인 경우 showLoadingView가 호출됩니다. 이런 동작이 의도된 것인지, 혹은 조건의 변경이 필요한지 확인해야 합니다.

  3. applyFontColorAttributes 메서드는 선택된 뷰 컨트롤러의 탭 바 아이템에 폰트 색상 속성을 적용합니다. 이 방법이 올바르게 동작하는지 확인하고 없는 경우에 필요한 스타일을 적용해야 합니다.

  4. 코드 상 tabBarController에 대한 액세스 권한을 확인하고 필요 시 안전한 옵셔널 연산자 사용을 검토해야 합니다.

  5. 특정 인덱스 값이 하드코딩되어 있으므로 추후에 변경 시 코드 수정이 필요할 수 있습니다. 해당 값을 상수로 정의하여 관리하는 것이 좋습니다.

  6. 코드 중복을 줄이기 위해 공통으로 사용되는 부분을 메서드로 분리하고 재사용성을 높일 수 있습니다.

  7. 코드 주석을 추가하여 각 섹션이 어떤 동작을 수행하는지 명확하게 설명해야 합니다.

Copy link

Choose a reason for hiding this comment

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

이 코드 패치의 코드 리뷰를 요청하셨습니다.

  • showLoadingView 메서드는 동일한 작업을 수행하는 중복된 블록을 가지고 있습니다. 이러한 중복성을 제거하여 코드 사용성을 향상시킬 수 있습니다.
  • beforeIndex == 2 및 beforeIndex == 3 인 경우에만 selectedIndex == 0 여부를 확인하는 조건으로 나누어집니다. 이 조건들은 기능적으로 유사하므로, 하나의 if beforeIndex >= 2 && selectedIndex == 0 {}로 합칠 수 있습니다.
  • 몇몇 중괄호({})의 들여쓰기가 일제히 맞지 않습니다.
  • 에러 핸들링이 없어서 로딩 뷰가 화면에서 강제로 제거될 수 있습니다. 에러 처리를 추가해야 할 필요가 있습니다.