[BE] 김경규 오늘의 짝꿍은? #10

KoungQ · 2024-04-10T11:03:33Z

미션 실행 결과

[테스트 실행 터미널 캡쳐 이미지 파일 첨부]
테스트 실행 터미널

테스트

실행 결과

미션 실행 실패 케이스 존재 시, 실패한 이유 고찰해보기

정상 작동

기능 명세서

이름 입력 받기(read)
String을 , 기준으로 쪼개서 멤버 리스트화 (parseMembers)
한글 외에 다른 글자가 들어왔는지 확인 (checkHasNoEnglish): 만약 다른 글자라면 다시 입력
짝 인원 수 입력 받기(read)
입력값이 숫자인지 확인(checkInteger): 만약 숫자가 아니라면 다시 입력
멤버 리스트 크기 구하기(memberNumber)
적절한 짝 인원 수 가져오기(getSize)
"총 멤버 수 >= 짝 인원 수" 검사 (checkDataValidity): 만약 "총 멤버 수 < 짝 인원수"라면 다시 입력
원하는 결과가 나올 때까지 추첨을 진행 (executeDraw)
짝 추첨 (generateRandomGroups)
출력 (printResult)
추첨을 멈출 것인지 결정 (isBreak)

taeseokyang

잘하시네요... 많이 배우겠습니다!

taeseokyang · 2024-04-10T13:51:40Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

+        return maximumGroupSize;
+    }
+
+    public int checkInteger(String s) throws InvalidInputException {


변수명을 s가 아닌 의미 있는 이름이면 좋을거 같습니다!

앗 잠깐 s로 둔다는걸 까먹고 그대로 뒀네요..! 감사합니당 :)

taeseokyang · 2024-04-10T13:55:14Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

+        return maximumGroupSize;
+    }
+
+    public int checkInteger(String s) throws InvalidInputException {


checkInteger의 반환이 숫자로 변환한 값인게 어색한거 같습니다. 함수의 기능을 보면 , convertToInteger라는 함수 명이 더욱 적절한것 같습니다!

이 메서드가 Integer.parseInt로 감싸기 전에 입력 받은 값이 숫자인지 먼저 판별해서 에러 방지하는 메서드를 만들려고 했는데 파라미터 타입과 메서드 타입이 달라서 그렇게 보이게 된 것 같네요..
말씀대로 convertToInteger로 하면 두가지 다 통용할 수 있을 것 같습니당 감사합니다 :)

taeseokyang · 2024-04-10T14:02:58Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

+            try {
+                return checkDataValidity(memberCnt, size);
+            } catch (InvalidInputException e) {
+                System.out.println(e.getMessage());


잘못된 입력일시, 다시 입력 받도록 하였는데, 그럼 입력 받는다는 코드가 중복 됩니다! 또한 재입력 로직은 getSize라는 함수명과는 어울리지않는 로직인거 같습니다. 각각 상황에 따라 입력 받는 함수를 만들고, 그 함수 내에서 재입력과 같은 로직을 포함 시키는 것은 어떤가요? ( 각각에 함수에서 read() 함수 사용 )

안그래도 그 문제 때문에 초기엔 read를 메서드 내부에서 호출했었는데 테스트코드에서 인풋을 파라미터로 전달하는 걸 보고 밖으로 빼게 됐습니다..! getSize라는 이름에 맞게 리펙토링 하거나 메서드 명을 바꿔보도록 하겠습니다! 감사합니다 :)

(저는 테스트코드를 바꿨어요🤣)

찾아보니깐 Mockito 쓰는거같은데 그거 사용하셨나요?! 코드 한번 참고해볼게요!

@KoungQ
엇 아뇨 ㅎㅎ 저는 외부 라이브러리는 사용하지 않았습니다! 이번 미션의 목표는 자바 학습이 1순위라고 생각해서요🐣

아하 그럼 저도 자바로만 한번 해보겠슴당 감사합니다!!

taeseokyang · 2024-04-10T14:17:40Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

-        LeetsMateApplication app = new LeetsMateApplication();
-        app.run();
+    // 원하는 결과가 나올 때까지 추첨을 진행하는 함수입니다.
+    public void executeDraw(List<String> memberList, int size) throws InvalidInputException {


지금 코드도 좋지만, do~while문이 조건이 뒤에 있기때문에 가독성 면에서 안좋다는 의견도 있습니다.
때문에 아래와 같은 코드도 한번 보시면 좋을거 같습니다. while 없이 재귀로 해결하여 가독성은 좀 더 좋아질 수 있다 생각합니다.
`public void executeDraw(List memberList, int size) throws InvalidInputException {
System.out.println("\n오늘의 짝 추천 결과입니다.");

List<List<String>> result = generateRandomGroups(memberList, size); // 짝 추첨 printResult(result); // 출력 System.out.println("추천을 완료했습니다."); if (isBreak()) { break; } executeDraw(memberList, size); }`

초기엔 재귀로 코드를 짰었는데, 만약 사용자가 악의적으로 프로그램에 입력을 잘못 입력할 시엔 메모리가 계속 할당되어 스택 오버플로우가 발생할 수 있다는 생각도 했어서 넓은 의미로 루프문으로 고치게 되었습니다..!
근데 현재 상황은 그 정도까지 고려하는건 투머치라고 생각되네요.. 다음엔 문제 상황에 좀 더 알맞게 구성해보겠습니당 감사합니다!!

저도 재귀로 사용했는데요! 스택오버플로를 고려하기도 했지만 현재는 오버엔지니어링이라 판단해 제한을 두지 않았습니다. 재귀로 사용한다면 tryCount를 두어 입력 횟수에 제한을 두는 것도 방법일 것 같아요!

아하 역시나 투머치였군요..! 입력 횟수 제한 좋은 방법인 것 같아요 추가해보도록 하겠슴당 감사합니다!

ay-eonii

첫 미션 고생하셨습니다!
stream 사용에 익숙하신가 보네요! 커밋메시지도 좋아요🔥

여유가 있다면 클래스를 나누고 책임을 분리해보는 것도 좋을 것 같아요 😊

ay-eonii · 2024-04-10T14:53:55Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

+        return IntStream.range(0, (int) Math.ceil((double) memberList.size() / maximumGroupSize))
+                .mapToObj(i -> memberList.subList(i * maximumGroupSize,
+                        Math.min((i + 1) * maximumGroupSize, memberList.size()) // 마지막 팀에 인원이 가득 차지 않았을 때 범위를 넘어가지 않기 위해 min 연산
+                ))
+                .collect(Collectors.toList());


stream을 잘 사용하시네요! 👍
(int) Math.ceil((double) memberList.size() / maximumGroupSize) 가 무엇을 의미하는지 이해가 어려운데 변수명을 붙여주면 파악하기 수월할 것 같아요 😆

멤버를 몇 개의 리스트로 나눌건지 설정하는 변수였습니다..! 멤버 사이즈를 그룹 최대 크기로 나눈 값의 올림 값으로 설정해둔 값인데 다시 보니 해석하기 힘드네요..! 다음 부터는 변수로 따로 빼서 보기 편하게 관리하거나 주석을 더 자세히 달아두겠습니다! 감사합니다 :)

ay-eonii · 2024-04-10T14:54:27Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

+        StringBuilder sb = new StringBuilder();
+        for (List<String> teams : result) {
+            Iterator<String> iter = teams.iterator();
+            sb.append("[");
+            while (iter.hasNext()) {
+                sb.append(" " + iter.next() + " "); // 출력 포멧을 위한 공백 추가
+                if (iter.hasNext()) {
+                    sb.append("|");
+                }
+            }
+            sb.append("]\n");
+        }


StringJoiner 혹은 String.join을 알아보면 도움이 될 것 같아요 😆

넵 알아보고 수정해보도록 하겠습니다! 감사합니다 :)

ay-eonii · 2024-04-10T14:55:46Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

+
+    // validation
+    public void checkHasNoEnglish(String members) throws InvalidInputException {
+        if (!members.matches("^[ㄱ-ㅎㅏ-ㅣ가-힣]*$"))


좋은데요?👍 정규표현식을 미리 컴파일해두는 건 어떨까요? 😊

미리 컴파일 해둔다는 생각은 못해봤습니다..! 알아보고 고쳐보도록하겠습니다!! 감사합니다 :)

ay-eonii · 2024-04-10T14:56:27Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

+        if (memberCount < maximumGroupSize)
+            throw new InvalidInputException("[ERROR] 최대 짝 수는 이름의 갯수보다 클 수 없습니다. 다시 입력해주세요.");
+        else if (maximumGroupSize < 1)
+            throw new InvalidInputException("[ERROR] 최대 짝 수는 1보다 작을 수 없습니다. 다시 입력해주세요.");
+        return maximumGroupSize;


조건마다 예외를 던져주네요 ! 그렇다면 else if 중 else 는 필요없을 것 같아요 😆

아하 찾아보니 예외를 던지거나 얼리 리턴할 경우엔 else if, else 보단 다중 if가 가독성 면에서도 유지보수 면에서도 더 좋다고 하네요 앞으로 이런 케이스엔 더 깔끔하게 작성할 수 있도록 노력해보겠습니당 감사합니다!!

ay-eonii · 2024-04-10T14:58:51Z

src/main/java/leets/leets_mate/LeetsMateApplication.java

+                if (input.equals("y")) {    // 재구성
+                    System.out.println("--------------------------------");
+                    return false;
+                } else if (input.equals("n")) { // 고정
+                    System.out.println("자리를 이동해 서로에게 인사해주세요.");
+                    return true;
+                } else {    // 그 외의 잘못된 응답 처리
+                    throw new InvalidInputException("[ERROR] 응답은 y 혹은 n으로 입력해야 합니다.");
+                }


early return을 사용한다면 else if 또는 else 는 필요없겠군요 !

위와 마찬가지였네요 고쳐보도록 하겠습니다! 감사합니다 :)

KoungQ · 2024-04-11T08:43:40Z

코드 리뷰 적용 완료

KoungQ added 11 commits April 4, 2024 19:52

PR test

684f512

FEAT: 기능 완성

c6e7b55

FEAT: 출력 포멧 완성

fb7ebd0

REFACTOR: repeat 메서드 추출

3ce4826

CHORE: 불필요한 주석 제거

182bfef

FIX: 잘못된 값 입력 시 다시 입력을 받도록 로직 수정

de1ac84

FIX: 메인 로직의 예외 위임 처리

b7f4eea

REFACTOR: 예외 위임 -> 메서드 내부에서 예외 처리

3ed5e41

REFACTOR: 스택 오버플로우 방지를 위해 재귀 -> 루프

49e0872

CHORE: 메서드명, 주석 수정

c99bb4f

REFACTOR: 메서드 파라미터 수정

9a8d0af

KoungQ self-assigned this Apr 10, 2024

taeseokyang reviewed Apr 10, 2024

View reviewed changes

ay-eonii reviewed Apr 10, 2024

View reviewed changes

KoungQ added 2 commits April 11, 2024 17:40

REFACTOR: 코드 리뷰 적용

240ac0a

REFACTOR: Mockito를 이용한 테스트 코드 한정 BufferedReader 제어

a2d3301

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] 김경규 오늘의 짝꿍은? #10

[BE] 김경규 오늘의 짝꿍은? #10

KoungQ commented Apr 10, 2024

taeseokyang left a comment

taeseokyang Apr 10, 2024

KoungQ Apr 11, 2024

taeseokyang Apr 10, 2024

KoungQ Apr 11, 2024

taeseokyang Apr 10, 2024

KoungQ Apr 11, 2024

ay-eonii Apr 11, 2024

KoungQ Apr 11, 2024

ay-eonii Apr 11, 2024 •

edited

Loading

KoungQ Apr 11, 2024

taeseokyang Apr 10, 2024

KoungQ Apr 11, 2024

ay-eonii Apr 11, 2024

KoungQ Apr 11, 2024

ay-eonii left a comment •

edited

Loading

ay-eonii Apr 10, 2024

KoungQ Apr 11, 2024

ay-eonii Apr 10, 2024

KoungQ Apr 11, 2024

ay-eonii Apr 10, 2024

KoungQ Apr 11, 2024

ay-eonii Apr 10, 2024

KoungQ Apr 11, 2024

ay-eonii Apr 10, 2024

KoungQ Apr 11, 2024

KoungQ commented Apr 11, 2024

[BE] 김경규 오늘의 짝꿍은? #10

Are you sure you want to change the base?

[BE] 김경규 오늘의 짝꿍은? #10

Conversation

KoungQ commented Apr 10, 2024

미션 실행 결과

미션 실행 실패 케이스 존재 시, 실패한 이유 고찰해보기

기능 명세서

taeseokyang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ay-eonii Apr 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ay-eonii left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KoungQ commented Apr 11, 2024

ay-eonii Apr 11, 2024 •

edited

Loading

ay-eonii left a comment •

edited

Loading