레시피 리스트UI를 생성해보았습니다. #9
Conversation
UI 개발 작업은 스크린샷도 함께 넣어주시면 이해가 더 잘 될 것 같아요. |
|
|
||
| import UIKit | ||
|
|
||
| class SearchBar: UIView { |
There was a problem hiding this comment.
UI는 확장할 일이 없으니 final class로 작성 부탁드려요
| private let searchBar = UISearchBar() | ||
|
|
||
| var searchText: String? { | ||
| return searchBar.text |
|
|
||
| import UIKit | ||
|
|
||
| class RecipeListView: UIView { |
|
|
||
| class RecipeListView: UIView { | ||
|
|
||
| let collectionView = UICollectionView(frame: .zero, collectionViewLayout: UICollectionViewFlowLayout()) |
There was a problem hiding this comment.
collectionView를 internal로 선언한 이유가 있을까요?
There was a problem hiding this comment.
ViewController에서
recipelistView.collectionView.dataSource = self
recipelistView.collectionView.delegate = self
이런식으로 선언하다보니 캡슐화를 놓친거 같습니다 변경하겠습니다!
There was a problem hiding this comment.
delegate와 dataSource는 view에서 같이 구현
There was a problem hiding this comment.
@f-lab-barry 내일 오전 중으로 수정 커밋 올리겠습니다!
|
|
||
| private func configureCollectionView() { | ||
| let layout = UICollectionViewFlowLayout() | ||
| layout.itemSize = CGSize(width: UIScreen.main.bounds.width - 20, height: 200) |
|
|
||
| import UIKit | ||
|
|
||
| class RecipeListViewCell: UICollectionViewCell { |
|
|
||
| class RecipeListViewCell: UICollectionViewCell { | ||
|
|
||
| private let imageView = UIImageView() |
There was a problem hiding this comment.
네이밍이 좀 더 구체적이면 어떨까요? (예. recipeThumbnailView)
| imageView.topAnchor.constraint(equalTo: topAnchor), | ||
| imageView.leadingAnchor.constraint(equalTo: leadingAnchor, constant: 10), | ||
| imageView.trailingAnchor.constraint(equalTo: trailingAnchor, constant: -10), | ||
| imageView.heightAnchor.constraint(equalTo: heightAnchor, multiplier: 0.75), |
| titleLabel.heightAnchor.constraint(equalTo: heightAnchor, multiplier: 0.15) | ||
| ]) | ||
|
|
||
| titleLabel.font = .systemFont(ofSize: 16, weight: .bold) |
There was a problem hiding this comment.
폰트를 한군데서 정의해서 관리하면 어때요? 나중에 다른데서도 통일성있는 폰트를 사용할 수 있을 것 같아요.
| imageView.image = nil | ||
| } |
| imageView.contentMode = .scaleAspectFill | ||
| imageView.clipsToBounds = true |
There was a problem hiding this comment.
이건 한번만 선언해주면 되는 것 같은데 configure 메서드에 있기보다 setupUI로 이동하면 어떨까요?
| private func loadImage(from url: URL) { | ||
| URLSession.shared.dataTask(with: url) { data, response, error in | ||
| guard let data = data, error == nil else { return } | ||
| DispatchQueue.main.async { | ||
| self.imageView.image = UIImage(data: data) | ||
| } | ||
| }.resume() | ||
| } |
There was a problem hiding this comment.
ImageView 에 extension 으로 빠져있으면 다른 화면에서 재사용이 가능한 로직으로 보이는데
재사용할 수 있도록 빼놓는 건 어떻게 생각하시나요?
|
|
||
| import UIKit | ||
|
|
||
| class RecipeListViewController: UIViewController, RecipeListViewModelDelegate { |
There was a problem hiding this comment.
RecipeListViewModelDelegate는 extention RecipeListViewController 으로 확장해서 가독성을 높이면 어떨까요?
| extension RecipeListViewController: UISearchBarDelegate { | ||
|
|
||
| func searchBar(_ searchBar: UISearchBar, textDidChange searchText: String) { | ||
| if searchBar.text?.count == 0 { |
There was a problem hiding this comment.
String의 extenstion에 isBlank 프러퍼티를 추가해 trim 하고 비어있는지 검토하는 로직을 넣어보는건 어때요?
There was a problem hiding this comment.
코드에 이상은 없지만 불필요한 return 빼주시는게 swift 컨벤션에 더 맞을 것 같아요~
There was a problem hiding this comment.
[3c03347] 제거 했습니다!
| private var viewModel: RecipeListViewModel | ||
| private var recipes: [RecipeListItemViewModel] = [] | ||
| private let searchBar = UISearchBar() | ||
| private let recipelistView = RecipeListView() |
There was a problem hiding this comment.
| private let recipelistView = RecipeListView() | |
| private let recipeListView = RecipeListView() |
|
|
||
| private var viewModel: RecipeListViewModel | ||
| private var recipes: [RecipeListItemViewModel] = [] | ||
| private let searchBar = UISearchBar() |
There was a problem hiding this comment.
아까 첫 커밋에서 만든 SearchBar는 어디에 쓰는거에요? 여기서 UISearchBar를 쓰고있길래 여쭤봐요!
| searchBar.delegate = self | ||
| } | ||
|
|
||
| func didFetchRecipes(_ recipes: [RecipeListItemViewModel]) { |
| DispatchQueue.main.async { | ||
| self.recipes = recipes | ||
| self.recipelistView.collectionView.reloadData() | ||
| } |
There was a problem hiding this comment.
UI를 업데이트 시키기 위해 비동기 처리가 필요하다고 판단했습니다.
| func didFail(with error: Error) { | ||
| print("Error: \(error.localizedDescription)") | ||
| } |
There was a problem hiding this comment.
fetch에 실패할 때 호출하는 메서드라면 Result 타입으로 다루던걸 그대로 전달하면서 에러처리하면 어때요?
메서드를 두개로 나눌 필요가 없어보여서요!
There was a problem hiding this comment.
func fetchedRecipes(result: Result<[RecipeListItemViewModel], Error>) {
DispatchQueue.main.async {
switch result {
case .success(let recipes):
self.recipes = recipes
self.recipeListView.reloadCollectionViewData()
case .failure(let error):
print("Error: \(error.localizedDescription)")
}
}
}
이런식으로 그대로 전달하면서 에러처리 말씀하시는거 맞나요?
|
|
||
| extension RecipeListViewController: UICollectionViewDataSource, UICollectionViewDelegate { | ||
| func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int { | ||
| return recipes.count |
There was a problem hiding this comment.
각 섹션에는 recipes의 갯수 만큼 반환해야 된다 생각했습니다
| navigationController?.pushViewController(detailVC, animated: true) | ||
| } else { | ||
| let RecipeIDErrorAlert = UIAlertController(title: "오류", message: "해당 정보를 찾지 못했습니다.", preferredStyle: .alert) | ||
| let okAction = UIAlertAction(title: "OK", style: .default) { _ in |
| @@ -9,6 +9,7 @@ import UIKit | |||
|
|
|||
| final class RecipeListView: UIView { | |||
|
|
|||
| private let itemSize = CGSize(width: UIScreen.main.bounds.width - 20, height: 200) | |||
There was a problem hiding this comment.
private enum Metric {
static let itemSize: CGSize = .init(width: UIScreen.main.bounds.width - 20, height: 200)
static let topMargin: CGFloat = 10.0
}
Metrice.itemSize
| @@ -119,3 +108,17 @@ extension RecipeListViewController: UISearchBarDelegate { | |||
| interactor.searchRecipes(with: query) | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
// MARK: - RecipeListViewModelDelegate
GeonH0
force-pushed
the
feature/feedListView
branch
from
a485b9b to
3c03347
Compare
|
|
|