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

[feat] World 이벤트 관리에 Observer 패턴 적용 #13

Merged
merged 19 commits into from
Dec 2, 2024

Conversation

yymin1022
Copy link
Member

@yymin1022 yymin1022 commented Nov 30, 2024

#️⃣ 연관 이슈

📝 작업 내용

GameEvent 인터페이스를 통해 LevelStatsChangedEvent와 RabbitEvent, StatsChangedEvent를 정의하고 World에 적용하였습니다.
테스트코드에 문제가 있는데, 이부분은 아직 작업하지 않았습니다. 추후 해결되면 연결되는 PR 작성하겠습니다.

참고 이미지 및 자료

Observer Pattern Structure

💬 리뷰 요구사항

건드리면 건드릴수록 테스트코드가 오버엔지니어링이 되기도 하고, 문제가 쌓이고 쌓여가는 느낌이네요.
가상의 이벤트를 구현하는 TestGameEvent 클래스가 말썽인데, test 경로 내에 있어서 그런 것 같아 다른 디렉토리에 정의하려고 합니다.
생각해둔것은 MockGameEvent로 이름을 변경하고, 테스트코드에서 참조하도록 하려고 하는데, 올바른 방향일까요? 더 좋은 방법이 있다면 의견 부탁드립니다.

@yymin1022 yymin1022 linked an issue Nov 30, 2024 that may be closed by this pull request
11 tasks
@yymin1022 yymin1022 self-assigned this Nov 30, 2024
@yymin1022 yymin1022 added the feat 기능 구현 및 디자인 패턴 적용 label Nov 30, 2024
@yymin1022 yymin1022 linked an issue Nov 30, 2024 that may be closed by this pull request
11 tasks
Copy link

@ysndy ysndy left a comment

Choose a reason for hiding this comment

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

Observer 패턴과 테스트 적용해주신 내용 잘 확인했습니다!

한 가지 궁금한 점이 있어서 코멘트 남겨봅니다 ㅎㅎ
Observer 패턴에서 Subject 는 자신의 변경사항을 Observer 들에게 알리는 주체로 알고 있는데요, (이슈에 첨부에주신 그림에서의 Publisher 역할) GameEvent 인터페이스의 역할이 Subject 가 아닌 단순 데이터 객체가 아닌지에 대한 생각이 듭니다! 오히려 won(), lost() 등의 상태 변화 메서드가 호출될 때 fireEvent를 호출하는 LevelWinListenerAdapter 와 WorldStatsListenerAdapter 가 Subject에 더 가까운 느낌이랄까요..? 혹시 GameEvent를 Subject로 설정하신 이유가 있으실까요?

Copy link

Choose a reason for hiding this comment

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

기존 LevelWinListener 인터페이스와 새로 만든 GameEventManager 클래스를 연결하기 위해 Adapter 패턴을 활용하셨군요! 👍

typeListeners.remove(listener);
}

public void fireEvent(GameEvent event) {
Copy link

Choose a reason for hiding this comment

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

이 부분이 옵저버 패턴에서 옵저버들을 업데이트 하는 부분이군요! 옵저버들의 세부 구현체가 아닌 인터페이스를 참조해서 잘 구현된 것 같습니다.

@yymin1022
Copy link
Member Author

Observer 패턴과 테스트 적용해주신 내용 잘 확인했습니다!

한 가지 궁금한 점이 있어서 코멘트 남겨봅니다 ㅎㅎ Observer 패턴에서 Subject 는 자신의 변경사항을 Observer 들에게 알리는 주체로 알고 있는데요, (이슈에 첨부에주신 그림에서의 Publisher 역할) GameEvent 인터페이스의 역할이 Subject 가 아닌 단순 데이터 객체가 아닌지에 대한 생각이 듭니다! 오히려 won(), lost() 등의 상태 변화 메서드가 호출될 때 fireEvent를 호출하는 LevelWinListenerAdapter 와 WorldStatsListenerAdapter 가 Subject에 더 가까운 느낌이랄까요..? 혹시 GameEvent를 Subject로 설정하신 이유가 있으실까요?

Subject 정하는게 사실 엄밀히 따지자니 이래저래 애매한 부분이 있어서, 고민하다가 데이터클래스 자체를 지정하게 되었습니다.
각 Adapter에 두자니 결국 그 둘도 데이터클래스를 기반으로 구성되는 것이라, 최상위 역할인 데이터클래스 하나를 Subject로 지정하는 편이 낫겠다고 생각했습니다.

Copy link

@ysndy ysndy left a comment

Choose a reason for hiding this comment

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

Subject 정하는게 사실 엄밀히 따지자니 이래저래 애매한 부분이 있어서, 고민하다가 데이터클래스 자체를 지정하게 되었습니다.
각 Adapter에 두자니 결국 그 둘도 데이터클래스를 기반으로 구성되는 것이라, 최상위 역할인 데이터클래스 하나를 Subject로 지정하는 편이 낫겠다고 생각했습니다.

역시 용민님도 이부분에 대해 고민을 하셨군요..! 말씀하신대로 각 Adapter에 두자니 애매한 부분이 있고 해서 최상위 역할인 데이터 클래스를 Subject로 정하신 부분에 대해 동의합니다.
고생하셨습니다!!

Copy link

@win-luck win-luck left a comment

Choose a reason for hiding this comment

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

코드 모두 확인했습니다!
이전까지의 PR은 수정을 통한 패턴 적용이 메인이었는데, 리소스가 높은 옵저버 패턴을 적용하신다고 정말 고생하셨습니다:)

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.

[feat] World 이벤트 관리에 Observer 패턴 적용
3 participants