-
Notifications
You must be signed in to change notification settings - Fork 4
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] 액션 이력 조회 리펙토링 #141
[BE] 액션 이력 조회 리펙토링 #141
Changes from 4 commits
b137d11
0bd9b88
8e29fa9
ba0d49f
772fe47
938f24e
12f39e0
3b6076c
76951fc
5ae22e2
d5a2180
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,57 +1,27 @@ | ||
package server.haengdong.presentation.response; | ||
|
||
import java.util.ArrayList; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Set; | ||
import server.haengdong.application.response.ActionAppResponse; | ||
|
||
public record StepResponse( | ||
List<ActionsResponse> steps | ||
String type, | ||
String stepName, | ||
List<String> members, | ||
List<ActionResponse> actions | ||
) { | ||
|
||
public static StepResponse of(List<ActionAppResponse> actions) { | ||
if (actions.isEmpty()) { | ||
return new StepResponse(List.of()); | ||
} | ||
List<ActionsResponse> actionsResponse = new ArrayList<>(); | ||
Set<String> members = new HashSet<>(); | ||
ActionAppResponse firstAction = getFirstAction(actions); | ||
List<ActionAppResponse> group = new ArrayList<>(); | ||
group.add(firstAction); | ||
String currentActionType = firstAction.actionTypeName(); | ||
members.add(firstAction.name()); | ||
|
||
for (int i = 1; i < actions.size(); i++) { | ||
ActionAppResponse action = actions.get(i); | ||
String typeName = action.actionTypeName(); | ||
if (currentActionType.equals(typeName)) { | ||
if (typeName.equals("IN")) { | ||
members.add(action.name()); | ||
} | ||
if (typeName.equals("OUT")) { | ||
members.remove(action.name()); | ||
} | ||
group.add(action); | ||
continue; | ||
} | ||
actionsResponse.add(ActionsResponse.of(group, members)); | ||
currentActionType = typeName; | ||
group.clear(); | ||
if (typeName.equals("IN")) { | ||
members.add(action.name()); | ||
} | ||
if (typeName.equals("OUT")) { | ||
members.remove(action.name()); | ||
} | ||
group.add(action); | ||
} | ||
actionsResponse.add(ActionsResponse.of(group, members)); | ||
|
||
return new StepResponse(actionsResponse); | ||
public static StepResponse of(List<ActionAppResponse> actions, List<String> members, String stepName) { | ||
return new StepResponse( | ||
actions.get(0).actionTypeName(), | ||
stepName, | ||
new ArrayList<>(members), | ||
toActionsResponse(actions) | ||
); | ||
} | ||
|
||
private static ActionAppResponse getFirstAction(List<ActionAppResponse> actions) { | ||
return actions.get(0); | ||
private static List<ActionResponse> toActionsResponse(List<ActionAppResponse> actions) { | ||
return actions.stream() | ||
.map(ActionResponse::of) | ||
.toList(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
package server.haengdong.presentation.response; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
import server.haengdong.application.response.ActionAppResponse; | ||
import server.haengdong.application.response.ActionAppResponse.ActionType; | ||
|
||
public record StepsResponse(List<StepResponse> steps) { | ||
|
||
public static StepsResponse of(List<ActionAppResponse> actions) { | ||
List<StepResponse> steps = new ArrayList<>(); | ||
List<String> currentMembers = new ArrayList<>(); | ||
List<List<ActionAppResponse>> groups = createGroups(actions); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. c: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 복잡하다고 생각해요. 좀더 리팩터링하면 좋겠단 생각이 들지만 현재까지 진행된 사항도 훌륭하다고 생각해요. 이후에 dto 변경 가능성도 높다고 생각해서 이만해도 충분하다 생각해요! |
||
|
||
int billGroupCount = 0; | ||
for (List<ActionAppResponse> group : groups) { | ||
changeCurrentMembers(group, currentMembers); | ||
if (group.get(0).actionType() == ActionType.BILL) { | ||
billGroupCount++; | ||
} | ||
StepResponse stepResponse = StepResponse.of(group, currentMembers, billGroupCount + "차"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. c: ActionType.BILL이 아닌 IN, OUT 경우에도 stepName이 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. stepName은 제거하도록 하겠습니다. |
||
steps.add(stepResponse); | ||
} | ||
return new StepsResponse(steps); | ||
} | ||
|
||
private static List<List<ActionAppResponse>> createGroups(List<ActionAppResponse> actions) { | ||
List<List<ActionAppResponse>> groups = new ArrayList<>(); | ||
|
||
for (ActionAppResponse action : actions) { | ||
if (groups.isEmpty() || isActionTypeChange(action, groups)) { | ||
groups.add(new ArrayList<>()); | ||
} | ||
groups.get(groups.size() - 1).add(action); | ||
} | ||
return groups; | ||
} | ||
|
||
private static boolean isActionTypeChange(ActionAppResponse action, List<List<ActionAppResponse>> groups) { | ||
List<ActionAppResponse> currentGroup = groups.get(groups.size() - 1); | ||
return currentGroup.get(0).actionType() != action.actionType(); | ||
} | ||
|
||
private static void changeCurrentMembers(List<ActionAppResponse> group, List<String> currentMembers) { | ||
for (ActionAppResponse action : group) { | ||
if (action.actionType() == ActionType.IN) { | ||
currentMembers.add(action.name()); | ||
continue; | ||
} | ||
if (action.actionType() == ActionType.OUT) { | ||
currentMembers.remove(action.name()); | ||
} | ||
Comment on lines
+43
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분에 MemberAction Type으로 인원을 추가하고 제거하는 비지니스 로직이 녹아있다고 판단되는데 어떻게 생각하시나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 백호가 말씀하신 대로 |
||
} | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package server.haengdong.presentation.response; | ||
|
||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import java.util.List; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
import server.haengdong.application.response.ActionAppResponse; | ||
import server.haengdong.application.response.ActionAppResponse.ActionType; | ||
|
||
class StepsResponseTest { | ||
|
||
@DisplayName("이웃한 같은 타입의 액션들을 그룹화 하여 응답객체를 생성한다.") | ||
@Test | ||
void of() { | ||
List<ActionAppResponse> actions = List.of( | ||
new ActionAppResponse(1L, "망쵸", null, 1L, ActionType.IN), | ||
new ActionAppResponse(2L, "백호", null, 2L, ActionType.IN), | ||
new ActionAppResponse(3L, "감자탕", 10_000L, 3L, ActionType.BILL), | ||
new ActionAppResponse(4L, "인생네컷", 10_000L, 4L, ActionType.BILL), | ||
new ActionAppResponse(5L, "소하", null, 5L, ActionType.IN), | ||
new ActionAppResponse(6L, "웨디", null, 6L, ActionType.IN), | ||
new ActionAppResponse(7L, "망쵸", null, 7L, ActionType.OUT), | ||
new ActionAppResponse(8L, "백호", null, 8L, ActionType.OUT), | ||
new ActionAppResponse(9L, "노래방", 20_000L, 9L, ActionType.BILL) | ||
); | ||
|
||
StepsResponse stepsResponse = StepsResponse.of(actions); | ||
|
||
StepsResponse expected = new StepsResponse( | ||
List.of( | ||
new StepResponse("IN", "0차", List.of("망쵸", "백호"), List.of( | ||
new ActionResponse(1L, "망쵸", null, 1L), | ||
new ActionResponse(2L, "백호", null, 2L) | ||
)), | ||
new StepResponse("BILL", "1차", List.of("망쵸", "백호"), List.of( | ||
new ActionResponse(3L, "감자탕", 10_000L, 3L), | ||
new ActionResponse(4L, "인생네컷", 10_000L, 4L) | ||
)), | ||
new StepResponse("IN", "1차", List.of("망쵸", "백호", "소하", "웨디"), List.of( | ||
new ActionResponse(5L, "소하", null, 5L), | ||
new ActionResponse(6L, "웨디", null, 6L) | ||
)), | ||
new StepResponse("OUT", "1차", List.of("소하", "웨디"), List.of( | ||
new ActionResponse(7L, "망쵸", null, 7L), | ||
new ActionResponse(8L, "백호", null, 8L) | ||
)), | ||
new StepResponse("BILL", "2차", List.of("소하", "웨디"), List.of( | ||
new ActionResponse(9L, "노래방", 20_000L, 9L) | ||
)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IN, OUT 경우에는 차수 이름을 null, BILL인 경우에는 1차부터 올리는 방식으로 하는게 깔끔할 것 같아요. 하지만 오늘 팀 전체 회의에서 stepName을 관리하지 않는 방향으로 얘기가 나왔으니, 확정되는 방향으로 리팩터링 하면 될 것 같습니다! 👍 |
||
) | ||
); | ||
|
||
assertThat(stepsResponse).isEqualTo(expected); | ||
} | ||
|
||
@DisplayName("액션이 없으면 빈 스탭들이 만들어진다.") | ||
@Test | ||
void ofEmpty() { | ||
StepsResponse stepsResponse = StepsResponse.of(List.of()); | ||
|
||
assertThat(stepsResponse.steps()).isEmpty(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r: 필드 순서와 of 메서드의 매개변수 순서를 맞춰야 할 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
반영했습니다!