-
Notifications
You must be signed in to change notification settings - Fork 4
[HotFix] StampView Change to IVO pattern #14
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
Conversation
| cardAnimatonModel.isTapped.toggle() | ||
|
|
||
| if cardAnimatonModel.isTapped { // 카드 회전 연속을 위해서 if문 분리 | ||
| withAnimation(.linear(duration: durationAndDelay)) { |
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.
View handling은 View 에서 하는게 맞지만, 그렇게 구현될 경우 너무 지그재그의 플로우가 이어지는 것이 가독성과 플로우 판단에 좋지 않다는 것을 서로 동감함.
➡️ 결론 : View의 코드지만 효율적인 코드 작성과 가독성을 위해서 예외적으로 Observed에서 View 코드를 작성.
| } | ||
| } | ||
|
|
||
| struct CardAnimationModel: Identifiable { |
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.
이 모델의 경우에는 Identifiable protocol을 준수하는(따르는) 의미가 있을까요 .. ???? 🤔
ps. @insub4067 Model을 View struct 내에 정의한건 어떻게 생각하시는지 의견이 궁금합니다 ~
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.
추가로 제가 안에 정의한 이유는 밖에서 쓰이는 일이 없고 또한 밖에서 쓰이는 것을 막고자 내부에서 정의했습니다. 🤗
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.
StampView.CardAnimationModel() 로 호출하면 호출은 할 수 있지 않나요 .. !!
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.
앗 생각해보니 그렇군요... 외부에서 인스턴스 생성을 막을려면 어떤 방법이 있을까요..?
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.
그게 고민입니다 ... 허허 ...
우선생님 고견이 궁금하군요 ? @insub4067
Co-authored-by: Eunyeong Kim <unnnyong@gmail.com>
| } | ||
|
|
||
| struct CardAnimationModel: Identifiable { | ||
| fileprivate init() {} |
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.
@unnnyong 이런 형식으로 fileprivate 로 접근을 제한하는 방안은 어떤가요??
init에 fileprivate를 작용하면 외부 접근을 막아서 class 외부에도 정의해도 될 것 같기도 합니다!😃
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.
우와 너무 좋은걸요 ? 그러면 Observed에서 사용되는 Model은 같은 파일 내에서만 선언되는 패턴이 되는걸까요 ?
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.
@unnnyong 넵 그러는게 좋을것 같습니다. 각 뷰 마다 필요한 변수가 다르기에 오용을 막기 위해서 파일 내에서 선언되는 패턴이 맞다고 생각됩니다!😃
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.
우선 해당 struct는 iterate 을 위한 struct 가 아니기 때문에 identifiable 일 필요가 없는거 같습니다
이와 동일하게 저희 프로젝트에서 사용하는 모든 data struct 는 iterate 하지 않을 경우 상속이 필요 없음으로
폴더명을 identifiable에서 models로 변경할 예정입니다 (현재 제가 작업하고 있는 브랜치가 머지되면 이후에 수정할 예정입니다)
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.
그리고 Observed 와 Model은 파일을 나누돼 extention 을 통해 (현재 observed와 같이) 사용하고자하는 View 에서만 참조/ 호출 할수 있도록 하면 어떨까 싶습니다
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.
Observed와 Model을 extention으로 나누는 것은 좋은것 같습니다.
하지만 Observed에서만 호출 사용된다면 굳이 파일을 나눌 필요가 있을까요??
|
@E-know conflict 가 발생하였읍니다 .. |
|
Conflict 해결해주세요 :( |
|
@unnnyong @insub4067 Conflict 해결했습니다. 마지 시켜주세요! 🤗 |
No description provided.