Fix/poke logic structure #181
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 관계 생성 시, 1로 증가하는 것보다 생성 후, pokeService에서 찌른 후 반영하는 것이 바람직해보임 - 기능 자체를 "친구관계 생성"만 수행하도록 변경
- 찌르고 찔린 관계에 대해 너무 종속적인 파라미터이름과 쓰임을 최대한 객관적으로 쓸 수 있도록 변경 - 방향성을 지켜야 하는 경우, `~FromTo`를 통해 파라미터 순서에 대해 사용성 개선
- Dto 클래스에 `EqualsAndHashCode` 롬복 추가 : 같은 필드 값에 대해 Stream 내에서 `distinct` 연산 처리 시, 필드가 같을 경우 동일한 객체로 판단할 수 있도록
- filter 추가(`isReply` false인 내역만) - 더 이상 답신할 찌르기 없으면 안뜨는게 맞음
gunom
reviewed
Dec 19, 2023
| public List<SimplePokeProfile> getAllPokeMeHistory(User user) { | ||
| List<PokeHistory> pokedHistories = pokeHistoryService.getAllPokedHistoryOrderByMostRecent(user.getId()); | ||
| return pokedHistories.stream() | ||
| .filter(pokeHistory -> !pokeHistory.getIsReply()) |
There was a problem hiding this comment.
미리 reply가 false인걸 repository에서 들고오면 조금 더 좋지 않을까요?
There was a problem hiding this comment.
해당 repository 메서드가 재사용될 일은 없겠죠? ㅋㅋ킼ㅋ
| public PokeToMeHistoryList getAllPokeMeHistory(User user, Pageable pageable) { | ||
| Page<PokeHistory> pokedHistories = pokeHistoryService.getAllPokedHistoryOrderByMostRecent(user.getId(), pageable); | ||
| List<SimplePokeProfile> pokeToMeHistories = pokedHistories.stream() | ||
| .filter(pokeHistory -> !pokeHistory.getIsReply()) |
| .userId(pokedId) | ||
| .friendUserId(pokerId) | ||
| .pokeCount(1) | ||
| .pokeCount(0) |
There was a problem hiding this comment.
친구 관계는 무조건 1로 시작해서 applyCount를 굳이 facade에서 한번 더 호출할 필요 없이 1로 넣으면 좋을 거 같아요!
또 친구는 항상 양뱡향으로 생성되니까 단방향으로 만들지 말고 양방향으로 만드는 방식으로 고쳐도 되지 않을까 생각합니다!
There was a problem hiding this comment.
"친구는 양방향으로 생성된다는 facade가 몰라도 될 거 같다"가 이유!
There was a problem hiding this comment.
하긴 4줄의 코드가 한 줄로 줄어들기도 하는 장점이 있겟네요..!!
자칫 한 메서드 안에 더 여러 기능?(생성 & 1 추가 x2) 를 수행한다 생각해서 분리해봤는데
facade에 노출된다는 점도 있었네요..!
| List<PokeHistory> allOfPokeFromTo = pokeHistoryService.getAllOfPokeBetween(friend.getUserId(), friend.getFriendUserId()); | ||
| return allOfPokeFromTo.stream() | ||
| .map(poke -> getPokeHistoryProfile(user, friend.getFriendUserId(), poke.getId())) | ||
| // .distinct() |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 PR Summary
🌵 Working Branch
🌴 Works
🌱 Related Issue