Skip to content

Conversation

@khyaejin
Copy link
Contributor

@khyaejin khyaejin commented Dec 7, 2025

연관 이슈

작업 내용

  • 곡 생성 로직 개선

    • 동일한 title + artist 조합이 존재하면 신규 생성 대신 기존 엔티티를 업데이트하는 방식으로 개선
    • 앨범 이미지가 있을 경우 S3 업로드 후 albumImgUrl 반영
  • 곡-사람 연결 API 기능 확장

    • 기존 PerformanceSong가 존재하면 업데이트, 없으면 신규 생성
    • orderInPerformance 필드 추가 및 전달 시 바로 반영
    • SongPersonConnectRequestDtoorderInPerformance 필드 추가
  • 전반적으로 생성·연결 과정의 중복 생성 방지 및 데이터 일관성 개선

논의하고 싶은 내용

  • 기존 공연들에 대해 orderInPerformance 미적용 데이터 마이그레이션 필요 여부
  • 향후 Song 중복 판정 기준(title/artist 외 확장 여부)

@khyaejin khyaejin self-assigned this Dec 7, 2025
@khyaejin khyaejin added the feat 새로운 기능 또는 요구사항을 추가 label Dec 7, 2025
@github-actions
Copy link

github-actions bot commented Dec 7, 2025

개선 사항

  1. 코드 가독성 향상

    • 근거: orElseGet(() -> Song.builder().build())와 같은 사용은 코드의 의도를 명확히 전달하지 못합니다. 객체가 생성될 때 기본값으로 설정해야 할 필드들이 있다면 명시적으로 지정하는 것이 좋습니다.
    • 수정 제안: 상태를 명확하게 하도록 Song 객체의 기본값을 직접 설정하는 방식으로 수정하세요.
  2. 앨범 이미지 업로드 후 URL 저장 방식

    • 근거: String albumImgUrl을 지역변수로 선언하여 사용하는 것은 상태관리를 어렵게 할 수 있습니다. 객체의 필드를 업데이트 하는 것이 더 명확합니다.
    • 수정 제안: song.setAlbumImgUrl(s3Uploader.upload(...))로 변경하여, 앨범 이미지의 URL이 Song 객체에 제대로 저장되도록 만드세요.
  3. 중복 코드 제거

    • 근거: performanceSongsRepository.save(performanceSong) 호출이 두 번 발생합니다. 이는 코드 중복을 초래하여 유지보수를 어렵게 합니다.
    • 수정 제안: performanceSong.setOrderInPerformance(requestDto.orderInPerformance())를 수행 후, 한 번만 performanceSongsRepository.save(performanceSong)을 호출하도록 최적화하세요.

잘한 부분

  • 이해하기 쉬운 주석 사용: 각 블록에 대한 설명을 통해 의도를 잘 표현했습니다.
  • 업데이트 로직의 추가: 기존 엔티티를 업데이트하고 신규 생성하는 방식으로 개선하여 중복 데이터를 방지한 점이 좋습니다.

핵심 평가: 전반적으로 유용한 개선이 이루어졌으나, 가독성과 유지보수성을 높이기 위해 몇 가지 개선이 필요합니다. 코드 중복을 줄이고, 객체의 상태를 명확히 하는 방향으로 보완하세요.

@khyaejin khyaejin merged commit 61d2a66 into develop Dec 7, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat 새로운 기능 또는 요구사항을 추가

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[fix] 최종 점검 도중 발견된 이슈 해결

2 participants