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

박스오피스 [Step3] Kobe, yyss99 #102

Open
wants to merge 45 commits into
base: ic_9_kobingu
Choose a base branch
from

Conversation

yy-ss99
Copy link

@yy-ss99 yy-ss99 commented Aug 14, 2023

안녕하세요!!! 코비와 와이가 드디어!!!!!! step3 PR을 보내게 되었습니다!!😆
잘부탁드립니다! 웨더!(@SungPyo)


고민되었던 점

  • 모던 컬랙션 뷰를 사용하여 화면을 새로고침을 할 때 새로운 데이터가 들어올 경우 데이터를 업데이트 해주는 부분.

    • 화면을 땅겨서 새로고침 액션을 실행할 경우, 새로운 데이터가 들어오는 경우에도 불구하고 화면에 새로운 데이터가 새롭게 업데이트 되지 않는 문제점이 있었습니다.
      그래서 메커니즘을 파악하고 뷰를 새롭게 그려주는 방법에 대해서 다시 공부해보고 해결방법을 찾았습니다.
      • UICollectionView.CellRegistration 타입의 CellRegistration을 만들고 커스텀하게 UICollectionViewListCell 타입의 ItemListCell 이름으로 클래스를 만들어 줍니다.
        ItemListCell 내부에 updateWithItem(_:) 함수를 구현하여 이 함수로 새로운 데이터가 들어올 경우 아이템을 업데이트하게 합니다.
        UICollectionViewDiffableDataSource 타입의 데이터소스를 만들고 return 값으로 collectionViewdequeueConfiguredReusableCell(using:for:item:)을 활용하여 각각의 parameter에 맞는 값을 넣어줍니다.
        특히 using 파라미터에는 위에서 만든 cellRegistration을 넣어줍니다.
        이후 NSDiffableDataSourceSnapshot 타입의 스냅샷을 만들어 SectionIdentifierType에는 미리 만들어 놓은 Seciotn 열거형을 넣어주고 ItemIdentifierType에는 Item 구조체를 넣어주어 snapshot 인스턴스를 만듭니다.
        snapshot.append(Item.all)을 구현하고 snapshot 인스턴스와 dataSource 인스턴스를 활용하여 dataSource.apply(snapshot)을 구현하여 새로운 데이터가 들어올 경우 데이터를 업데이트해 줄 수 있도록 하였습니다.
  • CollectionListView에서 임의로 높이를 주기

    • 셀의 위아래 여백을 주기 위해 cell의 constraint를 잡았으나 Layout 오류가 발생 했습니다. 높이를 따로 지정해 주지 않았는데도 불구하고 Height의 값이 44로 잡혀 있다는 오류가 발생했습니다. 검색 결과, 자동적으로 cell 높이를 부여해준다는 사실을 알았습니다.

      NSLayoutConstraint.activate([
              listContentView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: 60),
              listContentView.topAnchor.constraint(equalTo: contentView.topAnchor, constant: 30),
              listContentView.trailingAnchor.constraint(equalTo: contentView.trailingAnchor),
              listContentView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor, constant: -30),
      
              movieRankStackView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: 30),
              movieRankStackView.topAnchor.constraint(equalTo: contentView.topAnchor, constant: 30),
              movieRankStackView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor, constant: -30)
              ])
    • 그래서 cell 자체의 height를 높은 priority 로 고정해주는 것으로 해결했습니다.
      cell에 접근해서 cell 자체의 height를 결정해주고 이 cellheight에 높은 priority를 줍니다. 자동적으로 잡혀있던 cellheight보다 code로 준 height가 적용됩니다.

let contentViewHeightConstraint = contentView.heightAnchor.constraint(equalToConstant: 100)
        contentViewHeightConstraint.priority = .defaultHigh

        NSLayoutConstraint.activate([
            contentViewHeightConstraint,
            listContentView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: 60),
            listContentView.centerYAnchor.constraint(equalTo: contentView.centerYAnchor),

            movieRankStackView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: 30),
            movieRankStackView.centerYAnchor.constraint(equalTo: contentView.centerYAnchor)
        ])

🙏 조언을 얻고 싶은 부분

  • 모던 컬랙션 뷰의 구성요소가 궁금합니다.
    • Diffable Data Source , Compositional Layout 이 두 가지만 충족된다면 모던 컬랙션 뷰라고 불릴 수 있는지 궁금합니다.
      아니라면 모던 컬랙션 뷰라고 불리기 위해서는 어떤 구성요소들이 꼭 필수적으로 들어가야 하는지 궁금합니다.
  • 저희가 API KEY를 숨긴 방법이 올바른 방법인지 궁금합니다.

📚 참고 링크

📃 snapshot()
📃 UICollectionViewListCell
📃 UICollectionView.CellRegistration
📃 UICollectionViewDiffableDataSource
📃 UICollectionViewLayout
📃 UICollectionViewCompositionalLayout
📃 UICollectionView
🎦 Modern cell configuration
🎦 Lists in UICollectionView
📃 Implementing Modern Collection Views

devKobe24 and others added 30 commits July 28, 2023 14:40
확장한 NetworkConfigurable 프로토콜 내부에 generateURLRequest 메서드를 구현하였습니다.
각각의 상황에 맞는 에러를 던져주는 NetworkConfigurableError 케이스를 수정하여 주었습니다.
API KEY를 가져오기 위하여 Bundle을 확장하고 내부에 API라는 이름의 계산 프로퍼티를 생성 및 구현하였습니다.
UICellConfigurationState 확장, 내부 구현을 위한 준비 작업.
STEP2 브랜치에 업데이트를 위한 Revert

This reverts commit 0a171d2.
DailyBoxOfficeList 옵셔널 타입의 item 변수 생성, configurationState 오버라이드, 새로운 item 값이 들어오면 item을 업데이트 해주는 updateWithItem 함수 생성 및 구현.
collection view layout 및 data source 구현을 위한 베이스 작업 진행중.
Kobe and others added 14 commits August 10, 2023 19:57
CustomListCell 내부에 listContentViewBottomContraint와 movieRankStackViewBottomContraint를 생성 및 구현하였습니다.
contentViewHeightContraint 생성, contentViewHeightContraint.priority를 high으로 설정, listContentView, movieRankStackView centerY로 수정.
CustomListCell 내부에 updateConfiguration 메서드에 numberformat 적용.
UICollectionViewDelegate를 채택하여 collectionView(_:, didSelectItemAt:) 메서드를 생성 및 구현하여 내부에 화면 전환 코드를 구현하였습니다.
@SungPyo
Copy link

SungPyo commented Aug 15, 2023

안녕하세요!!! 코비와 와이가 드디어!!!!!! step3 PR을 보내게 되었습니다!!😆 잘부탁드립니다! 웨더!(@SungPyo)

고민되었던 점

  • 모던 컬랙션 뷰를 사용하여 화면을 새로고침을 할 때 새로운 데이터가 들어올 경우 데이터를 업데이트 해주는 부분.

    • 화면을 땅겨서 새로고침 액션을 실행할 경우, 새로운 데이터가 들어오는 경우에도 불구하고 화면에 새로운 데이터가 새롭게 업데이트 되지 않는 문제점이 있었습니다.
      그래서 메커니즘을 파악하고 뷰를 새롭게 그려주는 방법에 대해서 다시 공부해보고 해결방법을 찾았습니다.

      • UICollectionView.CellRegistration 타입의 CellRegistration을 만들고 커스텀하게 UICollectionViewListCell 타입의 ItemListCell 이름으로 클래스를 만들어 줍니다.
        ItemListCell 내부에 updateWithItem(_:) 함수를 구현하여 이 함수로 새로운 데이터가 들어올 경우 아이템을 업데이트하게 합니다.
        UICollectionViewDiffableDataSource 타입의 데이터소스를 만들고 return 값으로 collectionViewdequeueConfiguredReusableCell(using:for:item:)을 활용하여 각각의 parameter에 맞는 값을 넣어줍니다.
        특히 using 파라미터에는 위에서 만든 cellRegistration을 넣어줍니다.
        이후 NSDiffableDataSourceSnapshot 타입의 스냅샷을 만들어 SectionIdentifierType에는 미리 만들어 놓은 Seciotn 열거형을 넣어주고 ItemIdentifierType에는 Item 구조체를 넣어주어 snapshot 인스턴스를 만듭니다.
        snapshot.append(Item.all)을 구현하고 snapshot 인스턴스와 dataSource 인스턴스를 활용하여 dataSource.apply(snapshot)을 구현하여 새로운 데이터가 들어올 경우 데이터를 업데이트해 줄 수 있도록 하였습니다.
  • CollectionListView에서 임의로 높이를 주기

    • 셀의 위아래 여백을 주기 위해 cell의 constraint를 잡았으나 Layout 오류가 발생 했습니다. 높이를 따로 지정해 주지 않았는데도 불구하고 Height의 값이 44로 잡혀 있다는 오류가 발생했습니다. 검색 결과, 자동적으로 cell 높이를 부여해준다는 사실을 알았습니다.
      NSLayoutConstraint.activate([
              listContentView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: 60),
              listContentView.topAnchor.constraint(equalTo: contentView.topAnchor, constant: 30),
              listContentView.trailingAnchor.constraint(equalTo: contentView.trailingAnchor),
              listContentView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor, constant: -30),
      
              movieRankStackView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: 30),
              movieRankStackView.topAnchor.constraint(equalTo: contentView.topAnchor, constant: 30),
              movieRankStackView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor, constant: -30)
              ])
    • 그래서 cell 자체의 height를 높은 priority 로 고정해주는 것으로 해결했습니다.
      cell에 접근해서 cell 자체의 height를 결정해주고 이 cellheight에 높은 priority를 줍니다. 자동적으로 잡혀있던 cellheight보다 code로 준 height가 적용됩니다.
let contentViewHeightConstraint = contentView.heightAnchor.constraint(equalToConstant: 100)
        contentViewHeightConstraint.priority = .defaultHigh

        NSLayoutConstraint.activate([
            contentViewHeightConstraint,
            listContentView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: 60),
            listContentView.centerYAnchor.constraint(equalTo: contentView.centerYAnchor),

            movieRankStackView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor, constant: 30),
            movieRankStackView.centerYAnchor.constraint(equalTo: contentView.centerYAnchor)
        ])

🙏 조언을 얻고 싶은 부분

  • 모던 컬랙션 뷰의 구성요소가 궁금합니다.

    • Diffable Data Source , Compositional Layout 이 두 가지만 충족된다면 모던 컬랙션 뷰라고 불릴 수 있는지 궁금합니다.
      아니라면 모던 컬랙션 뷰라고 불리기 위해서는 어떤 구성요소들이 꼭 필수적으로 들어가야 하는지 궁금합니다.

문서에 대충 나와있듯이 Compositional Layout과 Diffable DataSource를 사용해서 업데이트를 해봐라~ 라고 되어있는걸 보면 충분하다고 보여집니다. 그냥 부르기 나름 아닐까요 ㅎㅎ 주관적인 생각이지만 중요한 부분이라고 생각되진 않습니다.

  • 저희가 API KEY를 숨긴 방법이 올바른 방법인지 궁금합니다.

이전 스텝에서 이어지는 부분이긴 한데요! 예를들어 저같은 경우는 API KEY에 대한 정보를 알지 못합니다. 이런경우는 어떻게 하죠? + plist 는 배포이후 ipa에서 읽는게 가능한 것으로 알고있는데요. 이런 부분도 찾아보셨을까요?

📚 참고 링크

📃 snapshot() 📃 UICollectionViewListCell 📃 UICollectionView.CellRegistration 📃 UICollectionViewDiffableDataSource 📃 UICollectionViewLayout 📃 UICollectionViewCompositionalLayout 📃 UICollectionView 🎦 Modern cell configuration 🎦 Lists in UICollectionView 📃 Implementing Modern Collection Views

@@ -8,12 +8,163 @@
import UIKit

class ViewController: UIViewController {

var dateCountUpForTest: Int = 20230720
Copy link

Choose a reason for hiding this comment

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

어떤 의미를 가진건지 모르겠습니다..? 그리고 접근제한자가 다 빠진것 같아요.

Comment on lines +19 to +23
collectionView.delegate = self

self.fetchDate()
self.configureHierarchy()
self.initRefreshControl()
Copy link

Choose a reason for hiding this comment

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

self가 있는것과 없는것의 구분이 어떤건가요?


let urlRequest = URLRequest(url: url)

networkManager.getBoxOfficeData(requestURL: urlRequest) { (boxOffice: BoxOffice) in
Copy link

Choose a reason for hiding this comment

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

에러에 대한 처리도 할 수 있는 형태면 좋을 것 같습니다.

Comment on lines +55 to +57
urlComponents.queryItems = !urlQureyItems.isEmpty ? urlQureyItems : nil

let fullPathURL = isFullPath ? URL(string: baseURL) : urlComponents.url
Copy link

Choose a reason for hiding this comment

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

사실 이런 부분들은 if문으로도 사용할 수 있는데 삼항 연산자를 사용하신 특별한 이유가 있나요?

Comment on lines 81 to 117
func makeRankIntensity(rankIntensityData: String?, completion: @escaping (NSAttributedString) -> Void?) {
guard var rankIntensityData = rankIntensityData else { return }
let font = UIFont.systemFont(ofSize: 17.0)

if rankIntensityData.contains("-") {
rankIntensityData = rankIntensityData.replacingOccurrences(of: "-", with: "▼")

let attributeDownRankIntensity = NSMutableAttributedString(string: rankIntensityData)
attributeDownRankIntensity.addAttributes([
NSMutableAttributedString.Key.foregroundColor: UIColor.blue,
NSMutableAttributedString.Key.font: font as Any
], range: (rankIntensityData as NSString).range(of: "▼"))

completion(attributeDownRankIntensity)
} else if rankIntensityData.contains("0") {
rankIntensityData = rankIntensityData.replacingOccurrences(of: "0", with: "-")

let attributeStayRankIntensity = NSMutableAttributedString(string: rankIntensityData)
attributeStayRankIntensity.addAttribute(
.foregroundColor,
value: UIColor.black,
range: (rankIntensityData as NSString).range(of: "-")
)

completion(attributeStayRankIntensity)
} else {
rankIntensityData = rankIntensityData.replacingOccurrences(of: rankIntensityData, with: "▲" + "\(rankIntensityData)")

let attributeUpRankIntensity = NSMutableAttributedString(string: rankIntensityData)
attributeUpRankIntensity.addAttributes([
NSMutableAttributedString.Key.foregroundColor: UIColor.red,
NSMutableAttributedString.Key.font: font as Any
], range: (rankIntensityData as NSString).range(of: "▲"))

completion(attributeUpRankIntensity)
}
}
Copy link

Choose a reason for hiding this comment

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

이 함수는 잘 읽히지 않는데요!
rankIntensityData 에 대한 부분을 enum형태로 만들어서 정리한다면 좀 더 보기 좋은 코드가 되지 않을까 싶습니다.

Comment on lines +135 to +144
if oldAndNewMovieCategory == "신작" {
let attributeOldAndNewMovieCategory = NSMutableAttributedString(string: oldAndNewMovieCategory)

attributeOldAndNewMovieCategory.addAttributes([
NSMutableAttributedString.Key.foregroundColor: UIColor.red,
NSMutableAttributedString.Key.font: font as Any
], range: (oldAndNewMovieCategory as NSString).range(of: oldAndNewMovieCategory))

completion(attributeOldAndNewMovieCategory)
}
Copy link

Choose a reason for hiding this comment

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

"신작" 이 아니면 동작하는 부분이 전혀 없는 것 같은데, 기존과는 다르게 여기에 guard문을 사용하신건 또 아니네요...?
아직까지 기준을 잘 모르겠습니다..

Copy link

@SungPyo SungPyo left a comment

Choose a reason for hiding this comment

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

코비 와이 안녕하세요.

이번엔 좀 큰 PR이라 눈에 확확 띄는 부분만 리뷰를 진행했는데요.

해당 코멘트 확인해 보시고, 수정이 필요한 부분은 수정 부탁드립니다.

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.

3 participants