일기장 [STEP 2] Zion, Serena

[serena0720]

Step2

@stevenkim18
안녕하세요! 스티븐
Zion, Serena 팀 입니다!
다이어리 프로젝트 Step2 리뷰 잘 부탁드립니다😊

---

고민한 부분

🔥 ViewController에서의 DiaryCoreDataManager에 대한 의존성

• ViewController(이하 VC)는 VC를 띄우는 데 있어서 필요한 데이터가 아닌 다른 것들을 의존하게 된다면 재사용성이 많이 떨어질 수 밖에 없다고 생각합니다. 따라서 이를 위해 상위 타입인 AppManager를 만들게 되었습니다.

  하지만 과제를 추가적으로 진행해나가면서 VC에 진입할 때 마다 fetch한 데이터를 갱신하고, Delete기능들 등이 추가되면서 DiaryCoreDataManager를 직접적으로 VC에서 주입했다면 재사용성은 떨어져도 코드의 가독성 및 이후의 유지 보수 관련해서는 더 쉽게 이해할 수 있는 코드가 되지않을까? 라고 생각했습니다. 그렇게 생각한 이유는 DiaryCoreDataManager를 주입받지 않았을 때 발생하는 AppManager와의 많은 소통때문이라고 생각합니다.

  물론 결과물만 올바르게 나온다면 두가지 방법 모두 좋다고 생각합니다. 또한 두가지의 경우를 모두 지키는 코드를 작성하는 것은 매우 어렵다고 생각합니다. (하나를 잃으면 하나를 얻는 관계라고 생각하기 때문에) 코드를 작성하시면서 이와 같은 요소들(재사용성 및 가독성)에 대한 우선순위나 VC에 대한 의존성을 관리할 때 조금더 생각하거나 고려하시는 부분들이 있는지 여쭤보고싶습니다.

🔥 TableView delete 한 후 Scroll할 때의 Constraint 충돌

• 현재 TableView에서는 DiffableDataSource을 활용하여 Cell을 관리하고 있습니다. Cell을 Swipe 했을 때 존재하는 Delete 기능을 사용하여 Cell을 삭제하고 스크롤시 아래와 같은 오류가 발생했습니다.

2023-09-12 16:28:45.001423+0900 Diary[23292:2537329] [LayoutConstraints] Unable to simultaneously satisfy constraints.
	Probably at least one of the constraints in the following list is one you don't want. 
	Try this: 
		(1) look at each constraint and try to figure out which you don't expect; 
		(2) find the code that added the unwanted constraint or constraints and fix it. 
(
    "<NSLayoutConstraint:0x600001ee8820 V:|-(5)-[UIStackView:0x154928590]   (active, names: '|':UITableViewCellContentView:0x1549293b0 )>",
    "<NSLayoutConstraint:0x600001ee8870 UIStackView:0x154928590.bottom == UITableViewCellContentView:0x1549293b0.bottom - 5   (active)>",
    "<NSLayoutConstraint:0x600001ee9630 'UIView-Encapsulated-Layout-Height' UITableViewCellContentView:0x1549293b0.height == 0   (active)>"
)

Will attempt to recover by breaking constraint 
<NSLayoutConstraint:0x600001ee8870 UIStackView:0x154928590.bottom == UITableViewCellContentView:0x1549293b0.bottom - 5   (active)>

Make a symbolic breakpoint at UIViewAlertForUnsatisfiableConstraints to catch this in the debugger.
The methods in the UIConstraintBasedLayoutDebugging category on UIView listed in <UIKitCore/UIView.h> may also be helpful.

• 오류 내용을 분석한 결과 Cell이 화면에 보이지 않을 때 발생하는 Constraint(UIView-Encapsulated-Layout-Height)와 제가 부여한 Cell이 Constraints가 충돌하여 나타나는 오류로 보였습니다.
  따라서 해당 오류를 해결하기 위해서 bottomConstraint에 대한 Priority를 999로 낮춰서 constraint가 충돌하는 것을 해결할 수 있었습니다.

  파악한 오류의 원인만 봐서는 누구나 경험할 수 있는 오류라고 판단됩니다. 그렇게 생각한 이유는 화면에 보이지 않을 때의 셀의 제약조건을 height == 0으로 준다면 어떤 조건이라도 충돌이 발생할 수 있기 때문입니다. 해당 오류는 DiffableDataSource를 사용하여 Delete 동작을 사용했기 때문에 경험한 것인지 아니면 해당 오류가 발생하는 다른 이유가 있는 것인지 여쭤보고 싶습니다.

문제에 대한 질문: StackOverFlow
오류해결 커밋: (bc89e58)

🔥 CoreDataManager에 CRUD 구현

• CoreData를 관리를 담당할 CoreDataManager 객체를 만들어 CoreData와 관련된 로직처리를 하고자 하였습니다. CoreData의 CRUD 역할 중 create는 범용적인 코드로 하기 한계가 있다고 생각했습니다. Data별로 entity가 가지는 value는 변할 수 있기 때문입니다. 하여 DiaryCoreDataManager를 별도로 만들어 Diary라는 특정 Data를 위한 객체를 만들어 create역할을 맡기고자하였습니다.

  이때 한가지 의문이 들었습니다. create를 DiaryCoreDataManager에게 모두 넘겨야할지, 아니면 둘 다 create역할을 알고 있어야할지 고민이 되었습니다.

  처음엔 create의 역할을 CoreDataManager에서도 갖고 있어야한다고 생각하였습니다. CoreDataManager에서 구현된 create를 기반으로 DiaryCoreDataManager에서 구체적인 data에 맞춘 create 메서드를 구현해야한다 생각했기 때문입니다.

  예시 코드

  // CoreDataManager
  func create(key: String, value: Any) {
      context.setValue(value, forKey: key)
  }
  
  // DiaryCoreDataManager
  func createDiaryData(title: String, body: String, date: Double) -> DiaryEntity {
      let entity = DiaryEntity(context: context)
      
      create(key: "title", value: title)
      create(key: "body", value: body)
      create(key: "date", value: date)
      return entity
  }

  하지만 이런 경우 CoreDataManager에 불필요하게 중복 코드가 생기는 것인가에 대한 고민이 들었습니다. 하여 CoreDataManager에서 create 메서드를 삭제하고 create는 DiaryCoreDataManager에서만 구체적으로 구현하였습니다.

  현재 코드

  // DiaryCoreDataManager
  func createDiaryData(title: String, body: String, date: Double) -> DiaryEntity {
      let entity = DiaryEntity(context: context)
      
      entity.title = title
      entity.body = body
      entity.date = date
      return entity
  }

🔥 DiaryDetailViewController에서 delete시 save 중복

• CoreData의 content를 delete한 후 이를 context에 save하는 로직을 구현하였습니다. 이때 하기와 같은 에러가 발생하였습니다.

  [error] error: Mutating a managed object 0xaef4c17744feb12c x-coredata://AEDDFE8E-AD18-4A2B-85FE-C6974CD59837/DiaryEntity/p117 (0x6000013d75c0) after it has been removed from its context.

• 오류 원인을 save()의 공식 문서에서 알 수 있게 되었습니다.

  Always verify that the context has uncommitted changes (using the hasChanges property) before invoking the save: method. Otherwise, Core Data may perform unnecessary work.
  save메서드는 호출하기 전 context의 변동사항을 체크하는데, 변동사항이 없을 시 save를 호출하는 것은 불필요한 작업이 됩니다.
  AppleDeveloper - save()

  이를 기반으로 디버깅을 한 결과 delete 시 save가 연달아 두 번 호출되는 것을 확인할 수 있었습니다. 두번째 화면인 DiaryDetailViewController에서 delete와 save를 한 후 첫번째 화면으로 바로 넘기는데, 첫번째 화면이 띄워지는 과정에서 데이터를 fetch하고 save를 호출하고 있었습니다.

  이에 save 호출이 중복되지 않게 코드 수정을 하여 에러 발생을 방지하였습니다.

🔥 Protocol을 활용하여 중복코드 삭제 및 수평적 확장

• share와 delete 기능 관련 AlertController와 ActivityViewController 코드가 각 ViewController에서 중복되었습니다. 하여 중복코드를 삭제하면서 UIViewController에 Alert/Activity Controller의 역할을 확장시킬 수 있도록 하고자 하였습니다.

  각각 AlertControllerShowable, ActivityViewControllerShowable 프로토콜을 생성하였습니다. extension에 기본 구현을 함으로 코드의 중복을 삭제시킬 수 있었습니다.

  프로토콜을 사용하면 수평적 기능 확장이 가능하게 된다는 장점이 존재합니다. 하지만 무분별하게 확장사용하는 것을 제한할 수 있도록 where Self를 사용하여 UIViewController에서만 기능 확장 사용이 가능하도록 하였습니다.

🔥 DiaryDetailViewController에 entity 주입

• diary text를 작성하는 도중 앱이 background로 가는 경우 저장이 될 수 있도록 구현하고자 하였습니다. 이때 text가 있는 경우 새롭게 create를 하거나 update를 하여 변경된 내용을 저장하고자 하였습니다.

  저희는 entity의 유무로 create와 update의 기준을 나누었기 때문에, 앱 사용 도중 최초 저장을 하게 되면 create 로직을 타고 새로운 content를 생성하였습니다. 하지만 background에 있던 앱을 다시 foreground로 가져와 수정을 완료하여 저장하고자 할 때 entity가 없기 때문에 다시 create로직을 타는 문제가 생겼습니다.

  이를 해결하고자 기존의 DiaryCoreDataManager의 createDiaryData 메서드가 entity를 반환하도록 수정하여 create 시 DiaryDetailViewController에 주입해주었습니다. 이로서 text가 있는 경우 중간에 앱이 background로 가게되면 바로 entity를 create하여 새로운 entity를 DiaryDetailViewController에 주입하였습니다. 이렇게 하여 추가 수정을 완료하여 저장하게 되면 entity가 있다고 판단하여 update로직을 탈 수 있도록 하였습니다.

[stevenkim18]

isUpdate가 무엇을 의미하나요?

[serena0720]

entity의 유무를 기준으로 update를 할지, create를 할지 분기처리하였습니다!
하여 isUpdate가 true인 경우 create가 아닌 update를 타게 됩니다.

[stevenkim18]

계속 코드를 보는데 굳이 date까지 주입을 받아야 되나라는 생각이 드네요..

의존성을 주입 받는 기준이 무엇인가요?

[LeeZion94]

해당 객체는 ViewController이기 때문에 View를 보여주는 것이외의 다른 것들을 주입받거나 가지고 있을 필요가 없다고 생각했습니다.
주입받는 것이아니라 내부에서 가지고 사용한다면 그만큼 재사용성이 떨어지는 것이라고 생각합니다.

따라서 View를 보여주기 위한 date를 주입받아서 사용했습니다 스티븐 ☺️

[stevenkim18]

요기 Private 붙일 수 있을 것 같아요 ㅎㅎ

[stevenkim18]

ActivityViewControllerShowable과 AlertControllerShowable은 protocol을 사용하지 않고 그냥 UIViewController 익스텐션으로만 구현해줘도 되지 않았을까요?

protocol로 빼신 이유가 있으신가요?

[LeeZion94]

해당 ViewController가 위의 Controller들을 Show 할 수 있음을 명시하기위해 사용했습니다.
협업을 하게 된다면 다른 사람들이 만든 Extension을 전부 확인하고 사용할 수 없다고 생각합니다.
위와 같은 이유로 Extension의 사용을 지양했습니다. :)

[stevenkim18]

뭔가 여기도 코드를 계속 보는데... 범용적으로 만들 수 있지 않을까 싶네요.

매개변수로는 딕셔너리를 받아서 처리하면 좋을 것 같네요..

그리고 꼭 리턴 값이 필요한가요??

[LeeZion94]

return 값이 불필요하다 생각되어 CoreDataManager 및 CoreDataManager와 ViewController가 상호작용하는 방식 전면을 Refactoring했습니다. create와 관련된 코드도 범용적으로 변경했습니다 스티븐!

[stevenkim18]

여기는 String 익스탠션을 사용해도 좋을 것 같아요 ㅎㅎ

[LeeZion94]

convertDiaryData는 String의 역할이 아니라고 생각합니다 스티븐 ☺️
그 이유는 해당 로직은 특정 String을 받아서 과제에서 제시하고 있는 Separator인 "\n"을 받아서 Title, Body로 나누어서 이를 Tuple형식으로 변환하여 반환하고 있습니다. 이는 범용적으로 활용될 수 있는 부분은 아니라고 생각합니다.
따라서 extension으로 사용하지 않고 별도의 메서드로 분리하여 처리했습니다! 조언 감사합니다!