-
Notifications
You must be signed in to change notification settings - Fork 6
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
3주차 과제(이진혁) #15
base: main
Are you sure you want to change the base?
3주차 과제(이진혁) #15
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ Aggregate란, 여러 도메인을 하나로 합치는 과정을 의미합니다. | |
|
||
|
||
아래 페이지는 신입모집 어드민 페이지입니다. | ||
<img width="1062" alt="image" src="https://github.com/JNU-econovation/Spring_Hell_Study/assets/54030889/9cc9bc92-923d-4649-bb92-ac914f4f6ab5"> | ||
|
||
하나의 API 요청에 모든 데이터를 담을 수 있도록 구성해봅시다. | ||
|
||
|
@@ -57,4 +58,4 @@ Aggregate란, 여러 도메인을 하나로 합치는 과정을 의미합니다. | |
|
||
제출지 : [email protected] | ||
|
||
### 마감시간 : 2024:05:11/21:30 | ||
### 마감시간 : 2024:05:11/21:30 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,17 +2,23 @@ | |
|
||
import com.econovation.third_project.database.Database; | ||
import com.econovation.third_project.database.Registration; | ||
import com.econovation.third_project.dto.request.GetAdminPageRequest; | ||
import com.econovation.third_project.helper.AdminPageHelper; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.stereotype.Controller; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PostMapping; | ||
import org.springframework.web.bind.annotation.RequestBody; | ||
|
||
import java.util.List; | ||
import java.util.Map; | ||
|
||
@Controller | ||
@RequiredArgsConstructor | ||
public class AdminQueryController { | ||
private final Database database; | ||
private final AdminPageHelper adminPageHelper; | ||
|
||
// 예시 코드 | ||
@PostMapping("/registration") | ||
|
@@ -25,4 +31,10 @@ public ResponseEntity<Registration> getRegistration(String userId) { | |
return ResponseEntity.ok().body(database.getRegistration(userId)); | ||
} | ||
|
||
@GetMapping("/admin") | ||
public ResponseEntity<List<Map<String,Object>>> getAdminPage(@RequestBody GetAdminPageRequest request) { | ||
List<Map<String,Object>> response = adminPageHelper.setResponse(request.requestInfos()); | ||
return ResponseEntity.ok().body(response); | ||
} | ||
Comment on lines
+35
to
+38
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. 반환할 경우 Object쓰는건 안티패턴입니다. 구체적인 타입 명시나 제네릭을 사용하십쇼! 당신은 타입형 언어를 쓰고 있습니다. 으딜 자스같은 반환형을 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 다 만들기 귀찮아서 Object 사용했는데 죄송합니다... |
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package com.econovation.third_project.domains.desiredtime; | ||
|
||
import com.econovation.third_project.database.Database; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
public class GetDesiredTimeUseCase { | ||
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. 도메인이 가장 첫 단어로 들어가야 나중에 파일 많아질때 금방 찾습니다. 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. 항상 관슴적으로 동사를 먼저 적었는데 확실히 도메인 먼저 적으면 찾기 쉽겠네요 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. 오 그렇군요 |
||
private final Database database; | ||
|
||
/** | ||
* 면접 희망 시간 | ||
* ex) (0,1) → 4명, (2,5) → 2명, (3,4) → 0명 | ||
*/ | ||
public Map<List<Integer>, Long> execute() { | ||
return database.findAllDesiredTime().stream() | ||
.flatMap(desiredTime -> desiredTime.getDesiredTime().stream()) | ||
.map(array -> Arrays.stream(array).boxed().collect(Collectors.toList())) | ||
.collect( | ||
Collectors.groupingBy( | ||
list -> list, | ||
Collectors.counting() | ||
) | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package com.econovation.third_project.domains.path.service; | ||
|
||
import com.econovation.third_project.database.Database; | ||
import com.econovation.third_project.database.Path; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
|
||
import static java.util.stream.Collectors.groupingBy; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
public class GetPathSumUseCase { | ||
private final Database database; | ||
|
||
public Map<String, Long> execute() { | ||
return database.findAllPath().stream().collect( | ||
groupingBy(Path::getSupportPath, Collectors.counting()) | ||
); | ||
} | ||
Comment on lines
+15
to
+22
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. 좋긴 한데 저 도메인 조회에 누적 합 구하는 별도의 케이스가 맞나 싶긴 하네요 너무 구체적인건 아닌가 싶기도 하고. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package com.econovation.third_project.domains.personalinfor.service; | ||
|
||
import com.econovation.third_project.database.Database; | ||
import com.econovation.third_project.database.PersonalInformation; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
|
||
import static java.util.stream.Collectors.groupingBy; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
public class GetPersonalInfoUseCase { | ||
private final Database database; | ||
|
||
/** | ||
* 소속 학과 합계 | ||
*/ | ||
public Map<String, Long> execute() { | ||
return database.findAllPersonalInformation().stream().collect( | ||
groupingBy(PersonalInformation::getMajor, Collectors.counting()) | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package com.econovation.third_project.domains.registration.adaptor; | ||
|
||
import com.econovation.third_project.database.Database; | ||
import com.econovation.third_project.database.Registration; | ||
import com.econovation.third_project.domains.registration.out.RegistrationQueryPort; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Component; | ||
|
||
import java.util.Collection; | ||
|
||
@Component | ||
@RequiredArgsConstructor | ||
public class RegistrationAdaptor implements RegistrationQueryPort { | ||
private final Database database; | ||
|
||
@Override | ||
public Collection<Registration> findAllRegistration() { | ||
return database.findAllRegistration(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package com.econovation.third_project.domains.registration.domain; | ||
|
||
public enum Priority { | ||
WEB("WEB"), | ||
APP("APP"), | ||
AI("AI"), | ||
GAME("GAME"), | ||
IoT("IoT"), | ||
ARVR("AR/VR"); | ||
|
||
private String value; | ||
Priority(String value) { | ||
this.value = value; | ||
} | ||
|
||
String getValue() { | ||
return value; | ||
} | ||
} | ||
Comment on lines
+3
to
+19
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. 우선순위라고 우리는 대충 알고는 있지만 모르는 사람은 이게 뭔지 잘 모를 수 있기 때문에 주석을 이런 이넘이나 doc 파일같은 경우 주석을 잘 다는 습관을 달아봅시다. 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. 넵!! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package com.econovation.third_project.domains.registration.out; | ||
|
||
import com.econovation.third_project.database.Registration; | ||
|
||
import java.util.Collection; | ||
|
||
public interface RegistrationQueryPort { | ||
Collection<Registration> findAllRegistration(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package com.econovation.third_project.domains.registration.service; | ||
|
||
import com.econovation.third_project.database.Registration; | ||
import com.econovation.third_project.domains.registration.adaptor.RegistrationAdaptor; | ||
import com.econovation.third_project.dto.PrioritySumDto; | ||
import com.econovation.third_project.dto.request.GetAdminPageRequest; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
public class GetApplicantSumUseCase { | ||
private final RegistrationAdaptor adaptor; | ||
|
||
// 지원자 누저 합계 | ||
public int execute() { | ||
return adaptor.findAllRegistration().size(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package com.econovation.third_project.domains.registration.service; | ||
|
||
import com.econovation.third_project.database.Registration; | ||
import com.econovation.third_project.domains.registration.adaptor.RegistrationAdaptor; | ||
import com.econovation.third_project.dto.PrioritySumDto; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.Collection; | ||
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
@Service | ||
@RequiredArgsConstructor | ||
public class GetPrioritySumUseCase { | ||
private final RegistrationAdaptor adaptor; | ||
|
||
/** | ||
* 희망 분야 합계 (1순위, 2순위) | ||
*/ | ||
public PrioritySumDto execute() { | ||
Collection<Registration> registrations = adaptor.findAllRegistration(); | ||
Map<String,Long> firstPriority = registrations.stream().collect( | ||
Collectors.groupingBy(Registration::getFirstPriority, Collectors.counting()) | ||
); | ||
Map<String,Long> secondPriority = registrations.stream().collect( | ||
Collectors.groupingBy(Registration::getSecondPriority, Collectors.counting()) | ||
); | ||
Comment on lines
+22
to
+27
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. 이렇게 되면 2번 순회를 하는 것이기 때문에 한번에 끝내는 걸로 최적화할 수 있을 것 같습니다. Map<String, Long>[] result = registrations.stream().collect(
Collectors.teeing(
Collectors.groupingBy(Registration::getFirstPriority, Collectors.counting()),
Collectors.groupingBy(Registration::getSecondPriority, Collectors.counting()),
(first, second) -> new Map[]{first, second}
)
); 이런식으로 합칠 수도 있구요. forEach로 찾는 방법도 있구요. 고민해보시면 좋을듯? 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. 홀리쉣 teeing은 또 처음 보네요.. |
||
return new PrioritySumDto(firstPriority, secondPriority); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package com.econovation.third_project.dto; | ||
|
||
import java.util.Map; | ||
|
||
public record PrioritySumDto(Map<String,Long> firstPrioritySum, | ||
Map<String,Long> secondPrioritySum) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.econovation.third_project.dto.request; | ||
|
||
import java.util.List; | ||
|
||
public record GetAdminPageRequest( | ||
List<String> requestInfos) // 지원자 누적 합계, 소속 학과 합계 .... | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
package com.econovation.third_project.helper; | ||
|
||
import com.econovation.third_project.domains.desiredtime.GetDesiredTimeUseCase; | ||
import com.econovation.third_project.domains.path.service.GetPathSumUseCase; | ||
import com.econovation.third_project.domains.personalinfor.service.GetPersonalInfoUseCase; | ||
import com.econovation.third_project.domains.registration.service.GetApplicantSumUseCase; | ||
import com.econovation.third_project.domains.registration.service.GetPrioritySumUseCase; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Component; | ||
|
||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
|
||
@Component | ||
@RequiredArgsConstructor | ||
public class AdminPageHelper { | ||
private final GetApplicantSumUseCase getApplicantSumUseCase; | ||
private final GetPathSumUseCase getPathSumUseCase; | ||
private final GetPersonalInfoUseCase getPersonalInfoUseCase; | ||
private final GetPrioritySumUseCase getPrioritySumUseCase; | ||
private final GetDesiredTimeUseCase getDesiredTimeUseCase; | ||
|
||
private Map<String, Object> response = new HashMap<>(); | ||
|
||
/** | ||
* 요청에 따른 응답 설정 | ||
* @param requestInfos | ||
* @return | ||
*/ | ||
public List<Map<String,Object>> setResponse(List<String> requestInfos) { | ||
return requestInfos.stream() | ||
.map(req -> { | ||
checkRequestInfo(req); | ||
return response; | ||
}) | ||
.toList(); | ||
} | ||
Comment on lines
+31
to
+38
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. 이렇게 짤 경우 반환해야할 데이터가 기획적으로 추가해야 할경우 switch문을 추가로 까야할 수도 있습니다. 이런게 확장에 불리한 구조라고 하는데요. 이 부분 꼭 다시 수정해보시길 바랍니다. 주로 aggregate과정에서는 각 데이터를 조회하는 걸 클래스단위로 구분하는게 정석이긴 합니다. 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. checkRequestInfo 랑 같이 한번 리팩토링 해보도록 하겠습니다! |
||
|
||
private void checkRequestInfo(String requestInfo) { | ||
switch (requestInfo) { | ||
case "Applicant": | ||
response.put("ApplicantSum", getApplicantSumUseCase.execute()); | ||
break; | ||
case "Path": | ||
response.put("PathSum", getPathSumUseCase.execute()); | ||
break; | ||
case "PersonalInfo": | ||
response.put("PersonalInfoSum", getPersonalInfoUseCase.execute()); | ||
break; | ||
case "Priority": | ||
response.put("PrioritySum", getPrioritySumUseCase.execute()); | ||
break; | ||
case "DesiredTime": | ||
response.put("DesiredTimeSum", getDesiredTimeUseCase.execute()); | ||
break; | ||
default: | ||
throw new IllegalArgumentException("Invalid request info"); | ||
} | ||
} | ||
Comment on lines
+40
to
+60
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. 코드가 아주 깔끔합니다. 하지만 조금 더 개선해서 전략 패턴이나 이 내부 구현을 한번 더 추상화하면 더 좋을 것 같습니다. |
||
} |
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.
데이터를 주고 받는 데 있어서 Map은 지양하고 있는 데 응답 클래스로 Map을 사용한 이유가 있을까요??
https://www.inflearn.com/questions/626856/api-%EC%9D%91%EB%8B%B5%EC%9C%BC%EB%A1%9C-map-%EC%82%AC%EC%9A%A9%EC%9D%84-%EC%A7%80%EC%96%91%ED%95%98%EB%8A%94-%EC%9D%B4%EC%9C%A0