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

3주차 과제(이진혁) #15

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 요청에 모든 데이터를 담을 수 있도록 구성해봅시다.

Expand Down Expand Up @@ -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
Expand Up @@ -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")
Expand All @@ -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) {
Copy link
Collaborator

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

List<Map<String,Object>> response = adminPageHelper.setResponse(request.requestInfos());
return ResponseEntity.ok().body(response);
}
Comment on lines +35 to +38
Copy link
Collaborator

Choose a reason for hiding this comment

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

반환할 경우 Object쓰는건 안티패턴입니다. 구체적인 타입 명시나 제네릭을 사용하십쇼! 당신은 타입형 언어를 쓰고 있습니다. 으딜 자스같은 반환형을

Copy link
Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ DTO 다 만들기 귀찮아서 Object 사용했는데 죄송합니다...


}
14 changes: 11 additions & 3 deletions src/main/java/com/econovation/third_project/database/Database.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
package com.econovation.third_project.database;

import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
import java.util.*;

import org.springframework.stereotype.Component;

/**
Expand All @@ -11,6 +10,7 @@
*/
@Component
public class Database {
//key : userId
private final Map<String, Registration> registration = new HashMap<>();
private final Map<String, Path> path = new HashMap<>();
private final Map<String, PersonalInformation> personalInformation = new HashMap<>();
Expand All @@ -25,4 +25,12 @@ public void register(Registration registrationRequest) {
public Registration getRegistration(String userId) {
return registration.get(userId);
}

public Collection<Registration> findAllRegistration() {return registration.values();}

public Collection<PersonalInformation> findAllPersonalInformation() {return personalInformation.values();}

public Collection<Path> findAllPath() {return path.values();}

public Collection<DesiredTime> findAllDesiredTime() {return desiredTime.values();}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

@Getter
public class Registration {
// 희망분야
// 희망분야 (개발자, 디자이너, PM)
private String hopeField;

// 1지망
Expand All @@ -13,6 +13,7 @@ public class Registration {
// 2지망
private String secondPriority;


public Registration(String hopeField, String firstPriority, String secondPriority) {
this.hopeField = hopeField;
this.firstPriority = firstPriority;
Expand Down
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 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

도메인이 가장 첫 단어로 들어가야 나중에 파일 많아질때 금방 찾습니다.

Copy link
Author

Choose a reason for hiding this comment

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

항상 관슴적으로 동사를 먼저 적었는데 확실히 도메인 먼저 적으면 찾기 쉽겠네요

Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

우선순위라고 우리는 대충 알고는 있지만 모르는 사람은 이게 뭔지 잘 모를 수 있기 때문에 주석을 이런 이넘이나 doc 파일같은 경우 주석을 잘 다는 습관을 달아봅시다.

Copy link
Author

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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로 찾는 방법도 있구요. 고민해보시면 좋을듯?

Copy link
Author

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 짤 경우 반환해야할 데이터가 기획적으로 추가해야 할경우 switch문을 추가로 까야할 수도 있습니다. 이런게 확장에 불리한 구조라고 하는데요. 이 부분 꼭 다시 수정해보시길 바랍니다. 주로 aggregate과정에서는 각 데이터를 조회하는 걸 클래스단위로 구분하는게 정석이긴 합니다.

Copy link
Author

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

코드가 아주 깔끔합니다. 하지만 조금 더 개선해서 전략 패턴이나 이 내부 구현을 한번 더 추상화하면 더 좋을 것 같습니다.
전략패턴을 사용할 수도 있구요.
그리고 코드에 값을 집어넣는 것 같은데check라는 동사가 맞을까 싶습니다. 차라리 set이 맞지 않나 아니면 add라던지

}