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

[Feat] 리액션 모아서 보내기 #246

Merged
merged 10 commits into from
Oct 3, 2024
Merged

[Feat] 리액션 모아서 보내기 #246

merged 10 commits into from
Oct 3, 2024

Conversation

hj1115hj
Copy link
Collaborator

@hj1115hj hj1115hj commented Sep 24, 2024

Issue

작업 내역 (Required)

  • detail, recomand ui에 밈 모아서 보내기 구현

  • 구현 로직

  • 첫번쨰 클릭으로 부터 1초 뒤에 400ms안에 클릭된 카운트 횟수를 서버로 전송하도록 함(병렬 처리를 위해 코루틴 사용)

  • 서버 데이터가 변경되기 까지 400ms 안에 클릭된 카운트 증가 못하도록 atomic 사용 (서버 fail시 마이너스할 카운트 횟수가 다를수 있어서.. )

  • 400ms안에 클릭되지 않은 이벤트 카운트는 1로 서버 업데이트 요청

  • debounce 적용된 component 재사용 가능하도록 isDebounceClick flag 설정함

Review Point (Required)

Screenshot

Before | After
https://github.com/user-attachments/assets/081e2470-f701-4a63-adbe-3419adbe5f78

관련 링크

@hj1115hj hj1115hj self-assigned this Sep 24, 2024
@hj1115hj hj1115hj changed the title [Feat] 밈 모아서 보내기 [Feat] 리액션 모아서 보내기 Sep 24, 2024
@hj1115hj hj1115hj closed this Sep 24, 2024
@hj1115hj hj1115hj reopened this Sep 24, 2024
@hj1115hj hj1115hj marked this pull request as draft September 24, 2024 16:19
@hj1115hj hj1115hj marked this pull request as ready for review September 26, 2024 15:09
Comment on lines -65 to +73
onClick = { multipleEventsCutter.processEvent(onClick) },
onClick = if (isDebounceClick) {
{ multipleEventsCutter.processEvent(onClick) }
} else {
{ onClick() }
},
Copy link
Member

Choose a reason for hiding this comment

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

singleClickable 모디파이어가 디바운스를 의도하고 만들어진 모디파이어인데 isDebounceClick이 추가된 이유는 무엇인가여

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ㅋㅋ버튼 연타할려면 디바운스 없어야해가지구 isDebounceClick 이 false면 디바운스 미적용 true면 적요으로 할려고 했숩니다! 기존에 있는 Clickable extension이랑 함께 쓸려고 flag로 구분함!

Copy link
Member

Choose a reason for hiding this comment

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

아예 그럼 별도로 디바운스가 없는 단순 clickable을 만드는게 더 좋아보여용, 아니면 기본 clickable을 사용하던가??
singleClickable에 isDebounceClick과 같이 디바운스 자체를 제어하는 파라미터는 있으면 안될 것 같은 개인적 생각..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오께이 리팩토링으로 잡겠습니다

Comment on lines +7 to +9
private val _isFirstClickEvent = AtomicBoolean(true)
private val _reactionCount = AtomicInteger(0)
private val _isUpdating = AtomicBoolean(false)
Copy link
Member

Choose a reason for hiding this comment

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

각각 타입들이 좀 특이한거같은데 설명 한번 부탁함당

Copy link
Collaborator Author

@hj1115hj hj1115hj Oct 1, 2024

Choose a reason for hiding this comment

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

private val _isFirstClickEvent = AtomicBoolean(true)

  • 맨 처음 ㅋㅋ버튼 클릭했을했는지 확인 하는 변수
  • 처음 viewModel 진입시는 true

private val _reactionCount = AtomicInteger(0)

  • 400ms 안에 연속으로 클리된 count횟수

private val _isUpdating = AtomicBoolean(false):

  • 서버가 업로드 하는동안 reactionCount 증가되는거 막기위해서 업로드 중인지 비교하는 변수
  • reactionCount가 증가되면 나중에 서버 실패시에 증가시킨 만큼 빼야되는데 그게 불일치할까마 막았음!

Atomic한이유는 코루틴을 병렬로 실행하면서 저기 있는 값들을 변경하는데 동시에 변경하려고 할때 안정성을 보장하려고 다른 코루틴이 읽거나 쓸때 접근하지 못하도록 lock건거입니다!
저거 안하면 병렬 타이밍 이슈로 클릭횟수보다 counting된게 적거나 많고 그러더라구욧

Copy link
Member

Choose a reason for hiding this comment

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

신기방기하구만유

Copy link
Collaborator

@EvergreenTree97 EvergreenTree97 left a comment

Choose a reason for hiding this comment

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

수고하셨슴다

reduce {
updateReaction(intent.meme) {
it.copy(
reactionCount = it.reactionCount - 1,
isReaction = false
reactionCount = it.reactionCount + 1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

서버에서 내려오는 토탈 memeCount로 적용된게 맞을까??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

웅 이부분은 연속 클릭아니여서 한번 클릭할때만 불려가지고+1, -1 연산 되도록 했엉

Copy link
Collaborator

@ze-zeh ze-zeh left a comment

Choose a reason for hiding this comment

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

수고했어용

@hj1115hj hj1115hj merged commit 1014e91 into develop Oct 3, 2024
@hj1115hj hj1115hj deleted the feat/kk_count branch October 3, 2024 12:50
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.

ㅋㅋㅋ 리액션 모아서 보내기
4 participants