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

일기장 [STEP2] 민쏜, 보리사랑 #53

Merged
merged 14 commits into from
Sep 5, 2022

Conversation

minsson
Copy link

@minsson minsson commented Aug 30, 2022

@ryan-son

안녕하세요, 라이언! ☺️
민쏜과 보리사랑입니다!

일기장 STEP 2 PR 보냅니다!
잘 부탁드립니다! 😁

감사합니다!


질문

1. 코어데이터 디렉토리 설정 - application support에 저장하는 방법

저희는 다음과 같이 어플리케이션의 support 디렉토리에 저장하고자 했습니다.

let container = NSPersistentContainer(name: "Diary")
let coreDataStoredUrl = FileManager.default.urls(
    for: FileManager.SearchPathDirectory.applicationSupportDirectory, 
    in: .allDomainsMask
)
let description = NSPersistentStoreDescription(url: coreDataStoredUrl[0])
container.persistentStoreDescriptions = [description]

다만 이 부분에 대해서 실제로 저장이 저부분에 되어있는지 확인 해보고 싶었는데 디버깅을 할 방법이 마땅치 않아서 검증을 할 수 없었습니다. 주소값 자체를 보니 0X12345 과 같은 식으로만 확인 되어서 어떤 영역인지 구분이 안되었습니다.

때문에 코드상에 아직 반영을 안했는데, 시뮬레이터 경로를 확인 하려면 직접 시뮬레이터가 저장된 mac 저장 경로를 들어가서 확인을 해야하는 걸까요?

2. Group By Feature 구성시 공통으로 사용되고 있는 파일들의 위치

기존의 Group By Function 구조의 file hierarchy에서 Group By Feature 형태로 변경해보았습니다.

다만 이런식으로 파일구조를 형성해보는게 처음이라서 헷갈리는 부분이 몇가지 있었습니다.

  • 현재 DiaryListViewControllerDiaryCreateViewController 등 프로젝트 전역적으로 사용되는 Model인 DiaryItem 타입을 어떻게 배치해야할지 잘 모르겠습니다. Common등의 네이밍에는 일기 관련 내용이 아닌 정말로 전반적으로 사용가능한 코드(Extension등)를 담는게 맞다고 생각하고 나니 일기 자체를 나타내는 DiaryItem을 마땅히 둘곳이 없다고 느껴졌습니다.
  • 코어 데이터 파일(Diary.xcdatamodeld)은 리소스에 위치하는게 맞다고 생각을 했는데 코어데이터 애플 예제를 확인해보니 feature마다 위치를 했습니다. 리소스 폴더 내부에서 파일 hierarchy를 갖는건 좋지 않은 접근일까요?

3. 코드를 작성시 코드 분리 수준 및 추상화 레벨

리스트 레이아웃의 콜렉션 뷰에서 trailing 스와이프 기능으로 공유와 삭제기능을 추가해주었습니다. 다만 삭제를 위해 UICollectionLayoutListConfiguration에 속성으로 trailingSwipeActionsConfigurationProvider을 지정해주었는데 deleteAction을 등록하는 과정에서 핸들러를 주입해주다 보니 코드가 길게 생성되었습니다.

저희는 "분리는 해보고 싶은데 이 한 부분을 위해서 3개의 액션과 AlertController 두개를 생성하는 메서드를 만드는게 가독성이 올라가는건가? 그냥 여기서 지금처럼 한번에 처리하는것도 상관없는건가? 라고 생각했습니다. 다만 이부분은 정답이 없다고 생각하기에, 라이언은 코드 작성하시면서 어떤 기준을 갖고 코딩하시는지 궁금했습니다.

4. 프로젝트의 전반적인 구조(OOP, POP 관련)

DiaryEditableViewController

DiaryListViewController의 일기 List에서 각 일기를 누르면 나오는 DiaryUpdateViewController와, 새 일기 생성을 누르면 나오는 DiaryCreateViewController의 각 루트뷰는 서로 매우 유사한 구성을 갖고 있습니다.

둘 다 텍스트뷰가 있고, 일기가 작성되거나 수정될 경우 코어데이터에 저장하기도 합니다.
따라서 완전히 동일한 중복 코드도 존재했습니다.

처음에는 프로토콜의 확장을 이용한 기본구현을 사용하려고 했습니다.
각 뷰컨트롤러마다 자신의 저장속성을 활용하는 () -> () 타입 메서드들이 있어, 조금 수정하여 파라미터로 주입받는 방향으로 바꾸면 될 것이라고 생각했습니다. 그런데 막상 리팩토링을 진행하다보니 아래의 contentTextView와 같은 저장속성들이 문제가 되었습니다.

    let contentTextView: UITextView = {
        let textView = UITextView()
        textView.translatesAutoresizingMaskIntoConstraints = false
        textView.keyboardDismissMode = .interactive
        textView.font = .preferredFont(forTextStyle: .body)
        return textView
    }()

프로토콜의 기본구현을 이용하자니 저장속성은 구현이 안 되는 걸로 알고 있고, 이걸 연산 프로퍼티로 바꾸는 것도 제가 못하는 건지 원래 안되는 건지 성공하지 못했습니다.

그러다 상속을 떠올리게 됐고, 두 뷰컨트롤러에서 완전히 중복되는 코드는 부모클래스를 만들어 구현하고, 상속 받는 것이 더 적절할 수 있겠다는 생각이 들었습니다. 불필요한 속성과 메서드를 상속받을 일이 없고, 상속의 상속이 이어질 일도 없어 상속의 단점 중 하나인 클래스가 불필요하게 무거워지는 문제도 상쇄된다고 생각했습니다.

하지만 상속으로도 모든 문제를 해결하지는 못했습니다.
DiaryUpdateViewController더보기(눌러서 공유, 삭제 옵션 선택 가능) 버튼을 DiaryCreateViewController에도 구현하고 싶어 부모클래스인 DiaryEditableViewController로 옮기려 했으나, DiaryUpdateViewControllerrightBarButtonActionSheetdeleteAlert를 옮기기 어려웠습니다.

해당 속성의 정의부를 보면 삭제 기능의 대상이 될 DiaryItem 타입이 필요한데, DiaryUpdateViewControllerDiaryCreateViewController에서 어떤 것을 줄지 지정할 수 없었기 때문입니다.

이와 관련 DiaryUpdateViewControllerrightBarButtonActionSheet를 메서드로 만들고, DiaryItemin-out 파라미터로 전달하려고 했으나 캡처 문제가 발생했고, 해결하지 못했습니다.

라이언이라면 이러한 상황에서 프로토콜의 기본구현을 활용했을지, 저희처럼 클래스의 상속을 활용했을지 궁금합니다.
또, 이런 상황이 되었을 때 구조/설계와 같은 큰 그림에서 어떻게 접근해야할지 궁금합니다.

5. 프로토콜 기본구현 또는 상속 시 접근제어 처리 방법

위 질문과 연관된 질문입니다. 프로토콜 기본구현 시에도, 상속 시에도 private 접근제어자를 사용할 수 없었습니다.
이런 경우 접근제어를 구현하기 위한 다른 방법이 있는지, 혹은 접근제어를 하지 않아도 무방한지 궁금합니다.

yusw10 and others added 11 commits August 24, 2022 12:29
- CoreData 설정
- DiaryItem 타입에 매핑될 DiaryEntity타입 구현
- CoreData로 CRUD기능을 수행할 DiaryCoreDataManager 싱글톤 패턴 구현
    - CRUD의 create 기능을 수행하는 saveDiary 메서드 구현
- CoreData에 uuid 추가
    - CoreData 내부의 DiaryEntity와 모델 객체인 DiaryItem을 연결하기 위함
    - saveDiary(diaryItem:)에 uuid 관련 수정 사항 반영
    - DiaryItem 타입에 uuid 추가
- CoreData로 CRUD기능을 수행할 DiaryCoreDataManager 싱글톤 패턴 구현
    - CRUD의 Update 기능을 수행하는 update(diaryItem:) 메서드 구현
    - CRUD의 Read 기능을 수행하는 fetchAllDiary() 메서드 구현
- "DiaryListViewController" rightBarButton에 DiaryCreateViewController로 push 하도록 수정
- "DiaryListViewController"로 push되었을 경우 TextView에 키보드를 자동으로 보여주도록 구현
- "DiaryListViewController" 키보드가 내려갈 시 자동으로 저장하도록 Notificatioin 등록
- "DiaryListViewController" 앱이 백그라운드로 넘어갈 시 자동으로 저장하도록 구현
- CoreData로 CRUD기능을 수행할 DiaryCoreDataManager 싱글톤 패턴 관련
    - CRUD의 Delete 기능을 수행하는 delete(diaryItem: DiaryItem) 메서드 구현
    - 기존 커밋 포함 Create, Read, Update, Delete 모두 구현 완료

"DiaryUpdateViewController"
- 네비게이션 바 오른쪽 버튼 추가
    - 삭제, 취소 버튼이 있는 Alert 추가
        - CoreData에서 DiaryEntity 삭제
    - 공유, 삭제, 취소 버튼이 있는 ActionSheet 추가
        - 공유 기능 추후 구현 예정
        - 삭제 버튼 터치시 삭제, 취소 버튼이 있는 Alert 띄우는 기능 구현
- rightBarButtonActionSheet의 공유 버튼 기능구현
    - UIActivityViewController를 활용해 조회중인 일기 상태를 공유할 수 있도록 구현
    - 현재 일기의 제목과 내용을 String타입으로 공유
- createTextViewContent 메서드 수정
    - 불필요한 DiaryItem 타입의 파라미터 삭제
    - 수정할 때 마다 추가 개행이 들어가던 오류 수정
- UIViewController를 상속 받는 DiaryEditableViewController 구현
    - DiaryCreateViewController와 DiaryUpdateViewController의 중복 코드를 해당 클래스에 구현
    - DiaryCreateViewController와 DiaryUpdateViewController가 해당 클래스를 상속 받고, 중복 코드는 제거
- 프로토콜 기본 구현이 아닌 상속을 선택한 이유
    - 프로토콜 기본 구현이 어려운 코드가 포함됨
    - 두 자식 뷰컨트롤러에게 불필요한 기능 없이, 공통 기능만 포함시킬 수 있어 상속의 단점이 상쇄됨
- 일기장 리스트 뷰의 각 행에 대해 공유와 삭제 기능을 위한 trailing swipe action 추가
    - 공유 선택시: 해당 일기의 공유를 위한 ActivityView(Share Sheet) 띄움
    - 삭제 선택시: 삭제 Alert 띄운 후 다시 삭제 선택시 해당 일기 완전 삭제
- "DiaryUpdateViewController"는 기존의 일기를 수정할 때만 생성되므로, diaryItem 변수의 불필요한 옵셔널 타입 제거
- "DiaryListViewController" 기존 일기 수정시 DiaryUpdateViewController에 데이터 전달하는 방식 변경
    - 이니셜라이저를 통해 데이터 전달
    - 기존 옵셔널 타입일 때 필요했던 코드 제거
- 기타 아규먼트 레이블 네이밍 변경
- 기존 DiaryListViewController에서 커스텀 컴포지셔널 레이아웃으로 리스트를 구현해, Border를 추가하기 위해 CALayer의 확장으로 메서드 구현
- 현재 UICollectionLayoutListConfiguration(appearance: .plain)을 활영하여 레이아웃을 구현해, Border가 더 이상 필요하지 않으므로 관련 코드 제거
@minsson minsson changed the base branch from main to ic_6_minsson September 2, 2022 07:36
Copy link

@ryan-son ryan-son left a comment

Choose a reason for hiding this comment

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

민쏜, 보리사랑, 리뷰가 조금 늦었네요. 죄송합니다! PR을 통한 리뷰 뿐만 아니라 회의실에서 Step 2에서 다루는CoreData와 여러분께서 궁금해하시는 내용에 대해 이야기 나눠봐요~

질의응답

  1. 코어데이터 디렉토리 설정 - application support에 저장하는 방법

직접 시뮬레이터 저장 경로로 이동해서 확인해보는 방법도 있고, Scheme > Run > Arguments에서 Arguments Passed On Launch의 argument를 아래와 같이 추가하여 경로를 확인하는 방법도 있습니다. 지금은 어느 경로에 저장되어 있는지 확인해보세요~

-com.apple.CoreData.SQLDebug 1

  1. Group By Feature 구성시 공통으로 사용되고 있는 파일들의 위치
  • 현재 DiaryListViewControllerDiaryCreateViewController 등 프로젝트 전역적으로 사용되는 Model인 DiaryItem 타입을 어떻게 배치해야할지 잘 모르겠습니다. Common등의 네이밍에는 일기 관련 내용이 아닌 정말로 전반적으로 사용가능한 코드(Extension등)를 담는게 맞다고 생각하고 나니 일기 자체를 나타내는 DiaryItem을 마땅히 둘곳이 없다고 느껴졌습니다.

각 Feature에서만 활용되는 모델이라면 해당 Feature Group에 두면 되고, 전역 또는 복수의 Feature에서 사용되는 모델이라면 SharedModel과 같은 단위를 만드는 것을 고려해볼 수 있습니다.

  • 코어 데이터 파일(Diary.xcdatamodeld)은 리소스에 위치하는게 맞다고 생각을 했는데 코어데이터 애플 예제를 확인해보니 feature마다 위치를 했습니다. 리소스 폴더 내부에서 파일 hierarchy를 갖는건 좋지 않은 접근일까요?

리소스에서 파일 관리를 하는 것은 생각해볼 수 있지만, 데이터 모델 파일이 복수 존재할 때 모두 Resource로 구분해두는 것은 어떤 Feature에 해당하는 데이터 모델인지 확인하기 어려울 수 있을 것 같아요. 여기에서는 Feature 단위에서 가지고 있는 편이 좋겠네요. 저의 경우에는 Resource에 이미지와 색상, information propetry list (info.plist), Launch screen 같은걸 넣어둘 때 사용하고 있어요.

  1. 코드를 작성시 코드 분리 수준 및 추상화 레벨

가독성은 개인차가 있기는 하지만 여러분처럼 정리의 필요성이 느껴지는 부분이 있다면 수정해보는 것도 좋을 것 같아요. 저는 가독성 향상이나 코드 정리를 위해 리팩토링할 때 어떤 모양으로 만들고 싶은지를 먼저 정해두고 작업하는 편입니다. 오늘 회의실에서 만나서 이야기 나눠봐요~

  1. 프로젝트의 전반적인 구조(OOP, POP 관련)

저라면 아래와 같이 생각해볼 것 같아요.

  1. 작성과 수정을 위한 ViewController는 명확히 다른 타입으로 나눌 필요가 있는지 판단한다.
  2. 나눌 필요가 있다면 공통적인 프로퍼티는 상속으로 묶고, 기능은 상속 또는 프로토콜 기본 구현을 이용하여 묶는다 (로직이 어떤 타입에 위치하는지에 따라 결정).

1 번은 하나의 ViewController를 재사용할 수 있도록 구성하는 것이 효율적일 때 고려해볼 수 있는 방법입니다. 지금과 같이 작성과 수정 화면이 디자인 및 기능 상 큰 차이가 없을 때, 그리고 디자인 변경이 생긴다면 작성과 수정 화면 모두에 동일하게 반영되어야 할 때 적용하면 좋습니다.
2 번은 UIKit 프레임워크가 상속을 통해 기능을 확장해나가는 구조를 가지고 있다는 점에 착안한 방법입니다. 공통 프로퍼티와 기능을 제외하고는 명시적으로 다른 타입을 만드는 것이므로 한 곳의 변경이 다른 곳에 영향을 미치지 않을 수 있다는 장점이 있으나, 이는 1 번에 비추어보면 단점이 될 때도 있습니다.

  1. 프로토콜 기본구현 또는 상속 시 접근제어 처리 방법

프로토콜에 명시한 요구사항은 최소한 프로토콜의 접근 수준 이상의 수준을 가져야합니다. 프로토콜의 자체적인 의미가 내부 구현이 어떻게 되더라도 최소한 해당 프로퍼티 혹은 메서드를 호출할 수 있음을 보장하는 것이기 때문입니다.

답변드린 내용과 Step 3, 보너스 스텝 내용을 포함해서 궁금하신 점이 있으시면 오늘 회의실에서 이야기 나누어봅시다~! 고생하셨어요!

Comment on lines -363 to +411
PRODUCT_BUNDLE_IDENTIFIER = "kr.yagom-academy.Diary";
PRODUCT_BUNDLE_IDENTIFIER = io.minsson;
Copy link

Choose a reason for hiding this comment

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

[P2]
타겟의 Bundle identifier는 앱을 고유하게 식별하기 위한 방법으로 다른 어떤 앱 또는 모듈과도 중복되지 않도록 설정해야 합니다. 만약 중복되는 Bundle identifier를 가진 타겟을 설치하면 기존에 동일한 Bundle identifier를 가진 앱이 덮어씌워지는 등 예상치 못한 불상사가 발생할 수 있어요. 자세한 내용은 아래 문서를 참고하세요.

Bundle identifier는 도메인 URL이 고유하다는 것에 착안해 Reverse domain name notation 방식으로 작명합니다. 수정하기 전 Bundle identifier는 무엇이었는지 확인해보세요.

  • com.domain.targetName
  • kr.co.domain.frameworkName
  • ...

Copy link
Author

Choose a reason for hiding this comment

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

그동안 신경쓰지 못했던 부분인데, 알려주셔서 감사합니다! 저희는 따로 도메인이 없다보니 임시로 io.minsson-borysarang.Diary로 변경해보았습니다!

import CoreData


public class DiaryEntity: NSManagedObject {
Copy link

Choose a reason for hiding this comment

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

코어데이터 모델 파일의 CodegenManual/None으로 선택하신 이유는 무엇인가요? 다른 옵션은 어떤 의미를 가지고 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

Generating Code에 따르면, Manual/None, Class Definition, Category/Extension. 세 가지 옵션이 있습니다.

Class Definition: 관리 객체 하위 클래스의 속성이나 기능과 Core Data가 생성하는 속성 파일을 편집할 필요가 없는 경우 클래스 정의를 선택

Category/Extension: 관리 객체 하위 클래스 내에 추가 편의 메서드 또는 비즈니스 논리를 추가하기 위해 선택

Manual/None: 관리 객체 하위 클래스의 속성을 편집하기 위해 선택 (예: 액세스 수정자를 변경하고 추가 편의 메서드 또는 비즈니스 논리를 추가).

저희는 코어데이터를 사용하는 게 처음이니만큼 직접 클래스의 속성을 편집해보며 원리를 이해하고자 Manual/None을 사용해보았습니다.

Comment on lines +55 to +60
static let sharedAppDelegate: AppDelegate = {
guard let delegate = UIApplication.shared.delegate as? AppDelegate else {
fatalError("Check to make sure the app delegate class hasn't changed: \(String(describing: UIApplication.shared.delegate))")
}
return delegate
}()
Copy link

Choose a reason for hiding this comment

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

[P3]
persistentContainer의 위치를 꼭 AppDelegate에 둘 필요는 없어요. CoreData를 다루는 다른 타입에 옮기면서 AppDelegate에 불필요한 코드를 정리해볼까요?

Copy link
Author

Choose a reason for hiding this comment

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

DiaryCoreDataManage로 옮겨주었습니다! 그런데 이는 객체의 역할 분리 관점에서 옮기는 거라고 보면 될까요?

Copy link

Choose a reason for hiding this comment

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

네, 지금과 같은 상황에서는 CoreData 기능을 제공하는 타입의 인스턴스가 굳이 AppDelegate 인스턴스에 의존할 필요가 없기 때문에 말씀드렸습니다~!

Comment on lines +11 to +17
class DiaryCoreDataManager {

static let shared = DiaryCoreDataManager()

private init() {

}
Copy link

Choose a reason for hiding this comment

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

코어데이터를 관리하는 타입을 싱글턴 패턴으로 구성하셨네요. 이렇게 구성하신 이유는 무엇인가요? 만약 단위 테스트가 추가된다면 테스트한 내용이 실제 앱에도 영향을 줄 것 같은데, 이 문제를 어떻게 해결할 수 있을까요?

Copy link

Choose a reason for hiding this comment

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

URL Session처럼 CoreData를 다루는 코드들도 일기 목록, 생성, 수정 등 전반적인 뷰 영역에서 호출되고 있기에, CoreData를 다루는 코드들을 묶어서 인스턴스화 하면 관리가 용이할것이라고 생각하여 싱글톤을 채택해 보았습니다. 다만 싱글톤을 사용할 경우 테스트 진행이 어렵다고 알고있었지만 이번 프로젝트를 진행하면서 테스트에 관련된 내용을 생각을 못했던것 같습니다.

코어데이터의 경우 persistentContainer를 인스턴스화 할때 어떤 엔터티를 가져올지를 지정해주어야 하는걸로 알고 있습니다. 현재는 코어데이터 관련 코드를 싱글턴 내부에 모아 두면서 persistentContainer를 인스턴스화 하는 코드도 싱글톤 내부에 존재합니다. 다만 이런 방식은 테스트와 실사용할 경우의 코어데이터를 분리할 수 없어 말씀하신대로 실제 앱에 영향을 끼치는 방식이라고 생각합니다.

이런 문제를 해결하기위한 방법으로 제가 생각한 방법은, 테스트를 위한 구조가 동일한 또하나의 코어 데이터 모델을 만들고 테스트 코드에서 이를 사용해(persistent 및 context 모두 주입하는 방식으로) 테스트를 해볼 수 있을것 같은데 맞는 접근일까요?

이전에 TDD를 실습하면서는 setup과 tearDown 메서드를 활용해서 싱글턴의 상태를 테스트 전 후가 같도록 설정해주었던것 같은데 더 나은 방법이 있을까요?

Copy link

Choose a reason for hiding this comment

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

말씀하신 방법대로 접근해볼 수도 있을 것 같고, NSPersistentContainer 인스턴스를 생성할 때 URL을 설정하여 inMemory 형태로 운용해보는 것도 생각해볼 수 있을 것 같아요.

if inMemory {
  container
    .persistentStoreDescriptions
    .first?
    .url = URL(fileURLWithPath: "/dev/null")
}

@minsson
Copy link
Author

minsson commented Sep 5, 2022

안녕하세요,라이언! ☺️

리뷰 감사합니다!
코멘트 남겨주신 내용과 더불어, 특히 회의실에서 보여주신 것들이 큰 도움이 됐습니다.
개념적인 단어들로만 듣던 이야기들을 코드 단계에서 살펴볼 수 있어 말로만 알던 개념을 더 잘 이해할 수 있게 되었습니다.

다시 한번 감사합니다! 🙇‍♂️

Copy link

@ryan-son ryan-son left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!

@ryan-son ryan-son merged commit b5af4c9 into yagom-academy:ic_6_minsson Sep 5, 2022
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