-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: 지도 탭 api 연결 #247
feat: 지도 탭 api 연결 #247
Changes from 9 commits
5b34e7a
eea935d
6653a8f
734d7a2
b07d399
58ae6e1
1e9a48c
c98c037
bb0dce4
7153470
4723e92
a73f4d0
d2068f1
3391f5a
68f1618
2a3c40c
0eb01e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ final class MapRouter: MapRoutingLogic, MapDataPassing { | |
let destination = playbackViewController.router?.dataStore | ||
else { return } | ||
destination.parentView = .other | ||
destination.index = source.index | ||
destination.index = source.postPlayStartIndex | ||
destination.posts = source.posts | ||
Comment on lines
34
to
36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 지난번 얘기한 대로 datapass 메서드로 분리되었으면 좋겠습니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저희 datapass 메소드 분리하나요? 👀 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. datapass 분리 (inout 사용) vs 그냥 참조해서 변경 두가지 의견이 있었던걸로 기억합니다 !! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 처음 Clean Swift의 Router의 의도가 데이터 전달 과 네비게이팅 분리라는 관점에서 저는 datapass분리도 괜찮다고 생각해요~ inout을 쓰면 순수함수를 잘 못지키는 것 같아 별로 쓰고 싶지 않았을 뿐, Clean Swift의 의도에 따라가려면 datapass가 더 적합한것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이전에 단순 참조해서 변경한 코드들이 있어서, 저도 datapass 메소드 분리하는 방향으로 수정해보겠습니다 :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아 이전에 https://clean-swift.com/pass-data-backward-more-elegantly-without-using-delegation/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아 저거는 메소드 보다는 상위 뷰컨트롤러 찾아서 passing하는 방법을 보고 공유했던거라 ㅋㅋㅋㅋ 밑에 메소드를 제대로 안봤었네용 |
||
viewController?.navigationController?.pushViewController(playbackViewController, animated: true) | ||
} | ||
|
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.
보니까 받는 데서는 결과를 쓰지 않는 모양인 것 같은데 그냥 반환 성공하면 true리턴?이 낫지 않나요
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.
추후에 Task 리턴을 들어내야하긴 하지만..?
테스트하는 입장에서 생각해보면 빈배열보다는 지웅님 말씀처럼 불리언 타입이 나을 것 같네용..
이참에 그냥 없애버릴까요..
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.
Test를 위해 Bool을 쓰긴 하는거니까요... 먼가 애매애매하군요.. 고민이 필요해 보입니다..!
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.
멘토님 말씀처럼 정답이 없다고 하셔가지고
-> 박터지는 싸움의 현장(어떻게 Task 블록으로만 감싸진 메서드를 테스트할 것인가)
논의가 필요할 것 같습니다.
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.
이건 추후 리팩토링 할 떄 의제로 남겨놓을게요 ADR ADR...