[FEAT] 게임 정보 수정 기능 추가

[Zena0128]

🌍 이슈 번호

• [FEAT] 경기 수정 #208

📝 구현 내용

• 게임 정보를 수정하는 api 추가

🍀 확인해야 할 부분

• 게임 정보 수정 시 일단은 라인업은 이미 존재하는 url 이용해 수정할 예정이라 라인업 외의 데이터만 변경하도록 구현
• 최대한 사용되고 있는 코드 구조나 구현/테스트 방식 등등 따라가면서 코드 작성하려고 노력은 했는데 부족한 부분이나 수정되어야 하는 부분 있으면 편하게 말씀 부탁드립니당 🙇
• 중간 커밋부터 이슈번호 붙이는 걸 잊어버렸네욥,,, 담부턴 꼭 붙이도록 하겠습니다..!

[Jin409]

처음인데도 깔끔하게 잘 해주신 것 같아요! 👍🏻 수고하셨습니다!

[Jin409]

이렇게 서비스단에서 dto 의 필드를 모두 getter 로 가져오니 game 의 캡슐화가 깨진다는 생각이 드는 것 같아요
dto 에서 엔티티로 변환하고 파라미터로 업데이트를 위한 엔티티를 넘기는 건 어떨까요?

[Jin409]

흠 근데 그렇게 하려면 수정에서는 게임팀 필드를 수정하지 않으니 GameTeam 을 null 로 초기화하는 생성자가 Game 에 추가되어야 하겠네요.. 이건 위험하려나요 @ldk980130 어떻게 생각하시는지 궁금하군용 🧐

[Jin409]

@fingersdanny 오빠는 이 부분 어떤 의견이야?

[Zena0128]

음 캡슐화..! 그렇군뇨
그러면 현재 GameRequestDto.Register 레코드 안의 toEntity 메소드와 같은 메소드를 Update 레코드에도 생성해서,
수정되지 않는 sport, idOfTeam1/2 같은 필드들은 getEntity에서 가져온 엔티티로부터 갖다쓰면 어떨까요..?!

[Zena0128]

업데이트에 필요한 필드들 + sport, manager, league 필드만 채워진 엔티티를 toEntity에서 리턴해서 해당 엔티티를 사용해서 업데이트하는 방식으로요!-!
그렇게 하면 game 엔티티의 필드들을 엔티티 내부 메소드에서 관리하게 돼서 서비스단에서 안 보이지 않을까용?

[hodadako]

@Jin409
1, 2번은 동의해

3. 엔티티의 역할이 뭐라고 생각하는지 궁금해.

4. 나도 같다고 생각하는데 Mapper를 다르게 구현하고 해당 반환 값을 넘기는 구조를 많이 봐왔어서.. update 자체는 서비스에서 하는게 맞는거 같아

[hodadako]

@fingersdanny 흠 저도 단순히 업데이트용으로만 엔티티를 새로 만든다는 점이 조금 찝찝하다고 생각은 했었어요

단순히 Game 내부 필드를 가려야 한다고 생각해서 위처럼 구현해야 한다면 난 반대야. 차라리 Entity에 Dto에 대한 의존성을 넣고 위의 update 메서드에는 request 만 집어넣을 수도 있어. (이렇게하면 다시 또 Entity가 Dto에 대한 의존성을 갖고 있는게 맞나? 라는 의문이 들 수 있고 실제로 DTO의 변경에 취약해지는 설계이니까 다른 문제가 생기겠지만.)

// GameService.java
@Transactional
    public void updateGame(Long leagueId, Long gameId, GameRequestDto.Update request, Member manager) {
        League league = entityUtils.getEntity(leagueId, League.class);
        if (!league.isManagedBy(manager)) {
            throw new UnauthorizedException(AuthorizationErrorMessages.PERMISSION_DENIED);
        }
        Game game = entityUtils.getEntity(gameId, Game.class);
        game.updateAll(request);
    }

// Game.java
    public void updateAll(GameRequestDto.Update updateDto) {
        updateName(updateDto.name());
        updateStartTime(updateDto.startTime());
        updateVideoId(updateDto.videoId());
        updateGameQuarter(updateDto.quarter());
        updateState(GameState.from(updateDto.state()));
        updateRound(Round.from(updateDto.round()));
    }

이런 식으로 구현하는 거 말씀하시는 거죠?? 흠 아니면 중간에 매퍼를 사용해서 게임 엔티티가 dto에 가지는 의존성을 중간에 한 번 끊는 방식은 어떻게 생각하시나요?!

// GameMapper.java
public class GameMapper {
    public static void updateGameFromDto(Game game, GameRequestDto.Update request) {
        game.updateName(request.name());
        game.updateStartTime(request.startTime());
        game.updateVideoId(request.videoId());
        // ...
    }
}

// GameService.java
    @Transactional
    public void updateGame(Long leagueId, Long gameId, GameRequestDto.Update request, Member manager) {
        League league = entityUtils.getEntity(leagueId, League.class);
        if (!league.isManagedBy(manager)) {
            throw new UnauthorizedException(AuthorizationErrorMessages.PERMISSION_DENIED);
        }
        Game game = entityUtils.getEntity(gameId, Game.class);
        GameMapper.updateGameFromDto(game, request);
    }

이런 식으로요! dto가 변경되면 매퍼도 변경되어야 하긴 하겠지만 엔티티에는 직접적인 영향이 없는 방향으로..!

접근해주신 방법은 좋은거 같아요. 다만 위처럼 구현한다면 보통 Mapper라는 이름이 붙은 객체를 객체 변환용으로 많이 사용하니까 @Jin409 승희가 코멘트 남긴것처럼 반환 값을 갖도록 하면 좋을거 같아요.

위처럼 구현한다면 반환 값은 Entity도 DTO도 아닌 제 3의 클래스여야 할텐데, 개인적으로는 이 이름을 ~Command 라고 사용하고 있어요.

[Zena0128]

@fingersdanny 피드백 감사합니다! 혹시 command 객체를 반환하게 되면 해당 객체는 어떠한 쓰임을 갖게 되는 건가요?
현재 훕치치에서는 업데이트 관련 메소드는 void로 구현한다고 이해하고 있어서 해당 객체의 용도를 잘 모르겠어요..

[Zena0128]

만약 중간에서 의존성 한번 끊어주는 객체를 사용하는 것도 반환값 때문에 애매해진다면
저도 엔티티가 dto에 의존하는 것보다는 차라리 처음 구현했던 방식처럼 dto 값들 빼와서 업데이트하는 게 더 좋아보이긴 합니다!
@Jin409 @ldk980130 다른 분들은 혹시 어떻게 생각하시는지용🤔

[Jin409]

저도 차라리 그게 나을 것 같아요!
dto 의 필드들이 꼭 entity 와 항상 동일한 것은 아니니 캡슐화를 완전히 저해하는 것도 아닐 것 같아서 괜찮을 거라고 생각이 드네요.
구현 속도를 고려해 논의는 이쯤 하고.. 업데이트용 엔티티가 괜찮은지에 대해 저는 개인적으로 더 공부해바야겠네요 ㅎㅎ 🤔