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

refactor: clubMember.size() -> countByClubClubTokenAndDeletedFalse 로 변경 #442

Conversation

I-migi
Copy link
Collaborator

@I-migi I-migi commented Nov 22, 2024

PR에 대한 설명 🔍

#435 (comment)

@I-migi I-migi requested a review from a team November 22, 2024 08:24
@I-migi I-migi self-assigned this Nov 22, 2024
@I-migi I-migi added the refactor✍️ 그냥 수정 label Nov 22, 2024
Copy link
Collaborator

@soeunnPark soeunnPark left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

@@ -37,6 +37,8 @@ public interface ClubMemberService {

void deleteAllClubMembers(String clubToken);

Integer countByClubClubTokenAndDeletedFalse(String clubToken);
Copy link
Collaborator

Choose a reason for hiding this comment

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

repository에 있는 메서드명이랑 동일하게 사용하기보다는 목적을 드러내는 메서드명이 어떨까요?
예를 들어, countExistingClub 등이 어떨까요? By~ 로 어떤 파라미터가 전달되는지 메서드명에 적게 되면, 의미가 중복되는 것 같아요. 뒤에 있는 String clubToken 파라미터 만으로도 어떤 것으로 조회를 하는지 알 수 있으니까요

@@ -148,6 +148,11 @@ public void deleteAllClubMembers(String clubToken) {
});
}

@Override
public Integer countByClubClubTokenAndDeletedFalse(String clubToken) {
return clubMemberReader.getClubMemberCountsByClubToken(clubToken);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Count는 불가산명사라서 뒤에 s를 붙일 필요 없어요

@I-migi I-migi merged commit 98ac1ec into develop Nov 22, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor✍️ 그냥 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants