-
Notifications
You must be signed in to change notification settings - Fork 147
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
프로젝트 매니저 [STEP 2] Jusbug #314
base: ic_9_jusbug2
Are you sure you want to change the base?
Conversation
|
||
final class CoreDataManager { | ||
|
||
static var shared: CoreDataManager = CoreDataManager() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추후에 변경될 여지가 있나요 ?
|
||
import UIKit | ||
|
||
final class CoreDataManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프로토콜로 지금 하는 crud역할을 나눈다면 어떤 모양이 될까요 ?
import CoreData | ||
|
||
@objc(Entity) | ||
public class Entity: NSManagedObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entity라는 이름을 모델이름으로 사용하실건가요 ?
let coreDataManager = CoreDataManager.shared | ||
var todoItem: [Entity] = [] | ||
var doingItem: [Entity] = [] | ||
var doneItem: [Entity] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
복수형으로 바꿔주세요.
|
||
import UIKit | ||
|
||
class MainViewController: UIViewController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 많이 들으셨겠지만, 상속여부에 따라 final키워드 붙여주세요.
private func alertActionForMove(sheet: UIAlertController, indexPath: IndexPath, to: Status, from: [Entity]) { | ||
let action = UIAlertAction(title: "Move to \(to.rawValue)", style: .default) { [weak self] action in | ||
guard let self = self else { return } | ||
let entity = from[indexPath.row] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let entity = from[indexPath.row]
의미하는바가 무엇일까요 ?
private let entity: Entity? = nil | ||
|
||
override func viewDidLoad() { | ||
configureTitle() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super콜은 필요없나요 ?
<rect key="frame" x="20" y="0.0" width="41.5" height="73.5"/> | ||
<subviews> | ||
<label opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251" text="Label" textAlignment="natural" lineBreakMode="tailTruncation" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="tzN-B2-blC"> | ||
<rect key="frame" x="0.0" y="0.0" width="41.5" height="20.5"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adjustsFontSizeToFit 이건 뭘 해주는걸까요 ?
D09389A72AC20FC5008A8304 /* CollectionReusableView.swift */, | ||
D0293F522ACAB3CE00505749 /* DateFormatManager.swift */, | ||
); | ||
path = ViewModels; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
뷰모델이 CollectionViewCell,CollectionReusableView,DateFormatManager가 맞나요 ?
let collectionView: UICollectionView | ||
let indexPath: IndexPath | ||
|
||
if let indexPathTodo = todoCollectionView.indexPathForItem(at: gestureRecognizer.location(in: todoCollectionView)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조금 가독성이 좋은 방향으로 나눠볼까요 ?
@July911
안녕하세요 줄라이!
요구사항을 완벽하게 구현하고 PR을 보내려 했지만 생각처럼 쉽게 진행되지 않아서 많이 답답했습니다.🥲
부족하지만 현재 구현을 마친 부분만이라도 먼저 리뷰를 받아 보고 싶어서 이렇게 PR을 작성했습니다.
꼼꼼한 지적 부탁드립니다.
고민했던 점
1. < popover 화면크기 >
🍏 문제상황
addButton
을 눌렀을 때secondVC
가popover
로 창이 띄어주도록 구현했으나 화면이 작아 안에 컴포넌트들이 가려지거나 정렬되지 않는 이슈가 있었습니다.🍎 해결방법
아이패드에서는 아이폰과 달리 화면이 크고 비율이 다르기에 직접 창의 위치와 사이즈를 설정해주어야 하는 문제가 있었고 직접
sourceView
,sourceRect
값을 설정하여present
되도록 수정하여 해결했습니다.2. < CollectionView 헤더 >
🍏 문제상황
예시처럼 컬렉션 뷰의 헤더 부분에 타이틀을 표시하게 설정하려면 따로 셀로 커스텀 해야할지 아니면 다른 방법이 있는지 고민하게 되었습니다.
🍎 해결방법
컬렉션 뷰의
SupplymentaryView
를 이용해서 헤더에 재사용 가능한 커스템 뷰를 넣어 헤더를 반환하는 형식으로 타이틀을 표시하도록 구현했습니다.3. < ActionSheet의 popoever 화살표 설정 >
🍏 문제상황
셀을 눌렀을 때 뜨는
ActionSheet
가popoever
되는 경우에는 선택한 해당 셀의 가운데 위치에서 생성되고 화살표 방향은.up
으로 설정하였습니다. 그러나 마지막 셀의 경우에는.up
으로 설정을 하게 되면 화면에 범주에서 벗어나 AlertAction의 메뉴를 선택할 수 없는 이슈가 있었습니다.🍎 해결방법
마지막 아이템의 경우에는 화살표 방향을
.down
으로 예외처리를 하여 이슈를 해결하였습니다.4. < Todo, Doing, Done 아이템 관리 >
🍏 문제상황
todo
,doing
,done
의 컬렉션 뷰의 정보들을 따로 관리하기 위해서 기존의 단일 코어데이터를 추가로 생성하여 따로 관리를 할지 아니면 각각 배열로 담아서 처리를 할지 고민하게 되었습니다.🍎 해결방법
3개의 컬렉션 뷰의 모든 기능은 동일하고 사용자 인식에 따라 역할이 달라지기 때문에 따로 코어데이터를 생성할 필요가 없다고 판단했고 각자 배열을 생성하여 관리할 수 있도록 수정하여 해결했습니다.
5. < AlertAction 관리 >
🍏 문제상황
컬렉션 뷰의
status
에 따라 보여지는 메뉴가 서로 상이하기 때문에 분기처리하여 메뉴를 생성해줘야하는데 이렇게 되면 코드가 너무 길어져 가독성과 효율이 너무 떨어지는 이슈가 있었습니다.🍎 해결방법
해당 셀을 이동시키고 삭제하는 AlertAction을 수행하는
alertActionForMove()
,alertActionForDelete()
메서드를 따로 구현하여status
와배열
을 전달인자로 받아와 해당 메서드를 호출함으로써 재사용성과 가독성을 높였습니다.해결하지 못한 점
CloudKit
을 이번에 코어데이터와 함께 사용하려고 했지만 프로젝트 진행이 늦어져 건드려 보지는 못했습니다.cell
을 선택했을 때는 해당 셀의 Entity의 정보를 가져와SecondVC
의 커스텀 이니셜라이져로 할당하여 뷰에 표시하고addButton
을 눌렀을 때는 Entity를 nil로 처리하여placeHolder
를 표시하도록 구현하려 했으나 어떤 이유때문인지 계속해서 초기화 에러가 발생하였습니다. 어떻게 해결하면 좋을지 궁금합니다.CollectionView
는TableView
와 달리SwipeAction
을 직접 지원하는 메서드가 없어 해결하지 못했습니다.조언을 얻고 싶은 점
MVVM
패턴 구조를 이론적으로는 이해했지만 실제 코드로 적용하려다 보니 쉽지 않았습니다.MVVM
패턴의 키포인트를 중점적으로 조언을 해주신다면 추후 자료를 더 참고하여 수정해보는 방향으로 진행을 해보겠습니다.