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

Review-1 #1

Open
OhKanghoon opened this issue Feb 11, 2019 · 6 comments
Open

Review-1 #1

OhKanghoon opened this issue Feb 11, 2019 · 6 comments

Comments

@OhKanghoon
Copy link
Member

OhKanghoon commented Feb 11, 2019

  • dequeueReusableCell forced cast 하는 부분을 좀 더 추상화해서 빼보기
  • NetworkData를 Protocol 에서 타입을 정하면 Service에 여러 네트워킹을 넣기 힘듭니다. Function 에서 Generic하게 받을 수 있는 방법을 생각해보세요
  • Netowkr Layer를 조금 더 추상화 해보기 ([url,params,method / 요청 / 결과 파싱] 분리하기)-> Moya를 사용해 보는 것도 괜춘
  • NetworkResult 는 Success(Data) 와 Failure(Error) 두개만 두고 error 를 enum 으로 처리해보기
  • 단순히 willDisplay에서 loadmore를 호출하면 중복호출될 가능성이 있으니 해결하기
  • Model 이름에서 VO 빼기
  • UserSearchResult 제거하고 NetworkResult로 통일해서 사용해보기
  • loadmore indicator는 테이블뷰 하단에 붙이기
@sujinnaljin
Copy link
Member

  • 단순히 willDisplay에서 loadmore를 호출하면 중복호출될 가능성이 있으니 해결하기

위 문제점이 잘 이해되지 않습니다. 어떤 상황에서 호출이 중복으로 될 수 있는지 설명해주실수 있나요?
또한 이문제를 해결하고자 할때 아예 다른 함수를 사용함으로써 접근해야하는지, 아니면 기존의 willDisplay 를 사용하되, 안에서 처리 로직을 따로 작성하면 되는지 궁금합니다.

@OhKanghoon
Copy link
Member Author

@sujinnaljin

  1. willDisplayCell 이 불렸을 때마다 네트워킹콜을 여러번 될 수 있다는게 첫번째 문제 (네트워킹 중에 스크롤 가능하기 때문) -> willDisplayCell을 사용과는 무관한 내용 / 추가 로직이 필요함 (필수)
  2. 굳이 willDisplayCell 로 index를 검사할 필요가 없음 didScroll 에서 offset으로도 어떤 부분에서 loadmore 해야하는지 파악할 수 있음 (선택)

@sujinnaljin
Copy link
Member

@OhKanghoon
답변 감사합니다. 그래서 셀의 마지막에서만 네트워킹 콜을 하기 위해 if indexPath.row == lastItemIdx 와 같은 조건을 작성하는 것 아닌가요? 통신이 모두 완료 된 후 20개의 데이터가 새롭게 패치 되면 아이템을 추가하고 테이블 뷰에 나타내는데 네트워크 중 스크롤과 어떤 관련이 있는지 잘 이해가 되지 않습니다. 네트워크 중 스크롤을 하면 willDisplayCell 의 if indexPath.row == lastItemIdx 이 조건 안으로 들어가서 네트워킹콜이 중복될 수 있다는 말씀 맞나요? 네트워크 중 스크롤하면서 로그를 프린트 해봐도 중복되게 출력되는 경우를 아직 발견하지 못해서 어떤 상황인지 잘 감이오지 않네요. 긴 글 읽어주셔서 감사합니다.

@OhKanghoon
Copy link
Member Author

OhKanghoon commented Feb 19, 2019

willDisplayCell 의 if indexPath.row == lastItemIdx 이 조건 안으로 들어가서 네트워킹콜이 중복될 수 있다는 말씀 맞나요?

  • 맞습니다. 여러번 호출되어 동일한 아이템이 20 + 20 (40)개씩 붙는 경우 확인했습니다

@sujinnaljin
Copy link
Member

네 다시해보겠습니다 감사합니다

@sujinnaljin
Copy link
Member

말씀해주신 부분 모두 수정 완료했습니다. 확인해주시면 감사하겠습니다. 👍

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

No branches or pull requests

2 participants