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] 7차 세미나 과제 #7

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[Feat] 7차 세미나 과제 #7

wants to merge 2 commits into from

Conversation

Chandrarla
Copy link
Collaborator

@Chandrarla Chandrarla commented Jun 7, 2024

🔍 What is the PR?

  • SOLID 원칙을 준수하며 MVC 패턴을 적용시켰습니다.
  • MovieRank 폴더의 View는 비즈니스 로직을 분리한 MVVM 패턴을 적용했습니다.

📍 PR Point

HomeViewController 분리

HomeViewController를 MVC 패턴으로 분리한 과정은 다음과 같습니다.

  1. HomeViewControllerrootViewHomeView()로 선언하며 View에 해당하는 부분을 HomeView()로 분리했습니다.
  2. HomeView()에는 2개의 CollectionViewnavigationView 컴포넌트가 있고, 각 컴포넌트 또한 별도의 파일로 분리했습니다.
[TopCollectionView, RecommendCollectionView, NavigationView]
▼
[HomeView]
▼
[HomeViewController]
final class HomeViewController: UIViewController {
    // MARK: - Property

    private let rootView = HomeView()
    
    // MARK: - Life Cycle
    
    override func loadView() {
        self.view = rootView
    }
    
    override func viewDidLoad() {
        super.viewDidLoad()
    }
...
final class HomeView: UIView {
    // MARK: - Component
    
    let topCollectionView = TopCollectionView()
    let recommendCollectionView = RecommendCollectionView()
    let navigationView = NavigationView()

    // MARK: - init
    override init(frame: CGRect) {
        super.init(frame: frame)
        setUI()
    }
    
    required init?(coder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }
...

MMVM 패턴 적용

  1. MVVM 패턴을 적용시킨 View는 Presentation > MovieRank에 있습니다.
  2. MovieRankViewController에서 구현한 기능은 버튼을 누르면 영화순위가 나오는 간단한 뷰입니다.
  3. 기존에 MVC를 적용한 MovieRankViewController에서 비즈니스 로직을 분리하여 MovieViewModel을 작성했습니다.

🙏 To Reviewers

  • MovieViewModel, MovieRankViewController가 MVVM 패턴을 적절하게 적용한 것이 맞는지 피드백 부탁드립니다.
  • CollectionView를 별도의 파일로 처음 분리해봤는데, Main > Views > TopCollectionView, RecommendCollectionView에서 두 CollectionView를 분리한 방식이 어떤지 피드백 부탁드립니다.

💭 Related Issues

@Chandrarla Chandrarla changed the title Feat/#7 [Feat] 7차 세미나 과제 Jun 7, 2024
Copy link
Member

@mcrkgus mcrkgus left a comment

Choose a reason for hiding this comment

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

확실히 코드가 많이 깔끔해져서 보기도 너무 좋네요 😊
폴더링도 코드도 MVC, MVVM 패턴을 잘 적용한 것 같습니다:-)

코드에 통일성이 있었으면 좋을 것 같아요. 예를들어 접근제어자 통일, 함수 분리, 네이밍 등등이요!
Extension 코드를 좀 더 활용하면 좋을 것 같습니다 ㅎㅎ

리펙토링 하시느라 너무 고생 많으셨습니다!! 앱잼 화이팅 🔥🔥

func bindUserId(id: String)
}

class LoginViewController: UIViewController {
Copy link
Member

Choose a reason for hiding this comment

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

final 키워드 붙여주면 좋을 것 같아요!

import SnapKit

final class LoginView: UIView {
// MARK: - Component
Copy link
Member

Choose a reason for hiding this comment

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

아래 컴포넌트들에 접근제어자를 따로 붙이지 않은 이유가 있을까요?
만약 특별한 이유가 없다면, private 접근제어자를 지정해주는게 좋을 것 같아요!

makeNicknameButton
)

idTextFieldRightView.addSubviews(idClearButton)
Copy link
Member

Choose a reason for hiding this comment

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

addSubView로 수정해주세용

pwTextFieldRightView.addSubviews(pwShowButton, pwClearButton)


titleLabel.snp.makeConstraints { make in
Copy link
Member

Choose a reason for hiding this comment

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

레이아웃을 잡는 부분은 따로 함수로 빼면 가독성이 좋아질 것 같아요!

func setLayout() {
 // 코드
}

Comment on lines +154 to +155
make.centerY.equalToSuperview()
make.centerX.equalToSuperview()
Copy link
Member

Choose a reason for hiding this comment

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

make.center.equalToSuperview()
로 해주셔도 똑같습니당

import UIKit

import SnapKit

Copy link
Member

Choose a reason for hiding this comment

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

import Then 해주세용

Comment on lines +34 to +38
required init?(coder: NSCoder) {
super.init(coder: coder)
self.backgroundColor = .clear
setUI()
}
Copy link
Member

Choose a reason for hiding this comment

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

required init에는 super.init(coder: coder)만 작성해주셔도 됩니당!

Comment on lines +13 to +19
var movies: [String] = [] {
didSet {
self.onMoviesUpdated?()
}
}

var onMoviesUpdated: (() -> Void)?
Copy link
Member

Choose a reason for hiding this comment

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

코드 넘 좋네용 😎

Comment on lines +26 to +28
DispatchQueue.main.async {
self?.movies = movieNames
}
Copy link
Member

Choose a reason for hiding this comment

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

DispatchQueue를 사용하신 이유가 있을까용?

setCollectionView()
}

private static func createFlowLayout() -> UICollectionViewFlowLayout {
Copy link
Member

Choose a reason for hiding this comment

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

static 키워드를 사용하신 이유가 무엇인가용??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7차 세미나 과제 #6
2 participants