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

[BE] feat#206#207 대기방 목록 조회 API 구현 및 기존 API들 리팩토링 #208

Merged
merged 8 commits into from
Nov 19, 2024

Conversation

NewCodes7
Copy link
Collaborator

@NewCodes7 NewCodes7 commented Nov 18, 2024

➕ 이슈 번호


🔎 작업 내용

  • 퀴즈 관련 API 리팩토링
    • 라인이 긴 함수들 분리 -> 가독성 향상
    • 퀴즈셋 CRUD 서비스 작업 유형 별로 분리 (한 파일 내 400 라인이 넘어서 분리 시도했음)
  • 퀴즈셋 검색 API 추가
  • 대기방 목록 조회 API 추가

🖼 참고 이미지

단위 테스트 통과

image

통합 �테스트 통과

image

🎯 리뷰 요구사항 (선택)

  • 많은 질문 환영합니다!

✅ Check List

  • merge할 브랜치의 위치를 확인했나요?
  • Label을 지정했나요?

@NewCodes7 NewCodes7 changed the title [BE] feat#206#207 대기방 목록 조회 API 구현 및 기존 API들 리팩토링 (진행중) [BE] feat#206#207 대기방 목록 조회 API 구현 및 기존 API들 리팩토링 Nov 18, 2024
Copy link
Collaborator

@DongHoonYu96 DongHoonYu96 left a comment

Choose a reason for hiding this comment

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

리팩토링 고생하셨습니다.
퀴즈셋 타이틀 설정시기에 대해 논의가 필요합니다!

Comment on lines +37 to +45
async findAllWithQuizzesAndChoices(
category: string,
offset: number,
limit: number,
search: string
): Promise<QuizSetList<QuizSetDto[]>> {
return this.quizSetReadService.findAllWithQuizzesAndChoices(category, offset, limit, search);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

P3 : 이러면 기존의 quizService를 주입받아 사용하는 다른 팀에서는 전부 코드를 수정해야 될것같네요
지금은 별 문제가 없지만, 큰 기업갔을때는 모든 다른 팀에 공문 뿌릴수도 없을거고...
다른팀에 영향을 주지 않으면서 리팩토링은 진행하는 방법은 없을까요?
찾아보니 Facade라는 것도 있네요.
지금은 오버엔지니어링 같기도 하네요

// 1. 인터페이스 정의
interface IQuizService {
  findAllWithQuizzesAndChoices(
    category: string,
    offset: number,
    limit: number,
    search: string
  ): Promise<QuizSetList<QuizSetDto[]>>;
  // ... 다른 메서드들
}

// 2. Facade 패턴 구현
@Injectable()
export class QuizService implements IQuizService {
  constructor(
    private readonly quizSetReadService: QuizSetReadService,
    // 기존 의존성들도 유지
    @InjectRepository(QuizSetModel)
    private readonly quizSetRepository: Repository<QuizSetModel>,
  ) {}

  // 새로운 서비스를 사용하는 구현
  async findAllWithQuizzesAndChoices(
    category: string,
    offset: number,
    limit: number,
    search: string
  ): Promise<QuizSetList<QuizSetDto[]>> {
    return this.quizSetReadService.findAllWithQuizzesAndChoices(
      category,
      offset,
      limit,
      search
    );
  }

  // 기존 메서드들은 그대로 유지
  async findOne(id: number): Promise<QuizSetModel> {
    return this.quizSetRepository.findOne({ where: { id } });
  }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

QuizService를 주입받는 쪽 코드인 controller에서 달라지는 건 없는데 구체적으로 어떤 부분을 말씀하시는 걸까요?

Comment on lines +28 to +35
rooms.push({
title: roomInfo.title,
gameMode: roomInfo.gameMode,
maxPlayerCount: parseInt(roomInfo.maxPlayerCount),
currentPlayerCount: currentPlayerCount,
quizSetTitle: roomInfo.quizSetTitle,
gameId: gameId
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

quizSetTitle을 어디서 설정할지 애매해서 일단 StartGame이 호출된뒤로 정해놓았습니다. (이때 db에서 퀴즈셋을 불러옴)
room이 만들어질때 사용자가 제목을 정하게 하는게 나을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

QuizSetTitle은 아래 경우에 수정하면 되지 않을까요?

  1. 대기방이 만들어지면서 디폴트 퀴즈셋이 정해질 때
  2. 대기방에서 호스트에 의해 퀴즈셋이 변경될 때

대기방 정보를 조회할 때 필요한거라 startGame이 된 이후에 저장하는 건 큰 의미가 없을 것 같아요

Copy link
Collaborator

@ijun17 ijun17 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 LGTM

Copy link
Collaborator

@songbuild00 songbuild00 left a comment

Choose a reason for hiding this comment

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

👍

@NewCodes7 NewCodes7 merged commit f42062a into boostcampwm-2024:dev-be Nov 19, 2024
4 checks passed
@NewCodes7 NewCodes7 deleted the feature-be-#207 branch November 19, 2024 04:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants