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

중복순위 처리버그 수정 #68

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

ipdae
Copy link
Member

@ipdae ipdae commented Oct 30, 2024

@ipdae ipdae requested a review from U-lis October 31, 2024 00:57
@ipdae ipdae self-assigned this Oct 31, 2024
Comment on lines +282 to +287
exist_avatar_addresses = [
i
for i, in db.query(WorldBossReward.avatar_address).filter_by(
raid_id=raid_id
)
]
Copy link
Contributor

Choose a reason for hiding this comment

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

이건 loop 밖에서 한번만 가져와도 될 것 같습니다. offset 을 딱히 사용하는 것도 아니고 full query 라서 반복할 필요가 없을 것 같네요.

Comment on lines +289 to +291
target_avatar_addresses = [
i for i in avatar_addresses if i not in exist_avatar_addresses
]
Copy link
Contributor

Choose a reason for hiding this comment

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

기왕에 list 만드느라 loop 도는거 exist_avatar_addressesavatar_addresses 를 set 으로 만들어서 avatar_addresses - exist_avatar_addresses 를 하면 빠르고 간편할 것 같습니다.

Comment on lines +294 to +295
else:
offset += payload_size
Copy link
Contributor

Choose a reason for hiding this comment

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

offset 을 계속 증가시키다가 DP 에서 가져오는 값이 없어지면 계속 else 를 타면서 무한 loop 에 빠질 수 있을 것 같습니다. DP 에서 가져올 게 더 없는 경우 탈출시키는 게 필요하지 않을까요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Hold
Development

Successfully merging this pull request may close these issues.

2 participants