[BE] 이강혁 👥 오늘의 짝꿍은? by hyxklee · Pull Request #6 · Leets-Official/leets-mate-java-1

hyxklee · 2024-04-10T05:09:06Z

미션 실행 결과

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

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

정상작동 확인

무작위로 짝궁을 추첨하는 generateRandomGroups 메서드에 대한 단위 테스트를 작성하는 게 좋을지 랜덤에 대한 단위 테스트는 무시하는 게 좋을지 고민 중임

기능 명세서

String으로 이름을 입력받아 [parseMembers] 메서드에서 ','를 기준으로 이름을 나눔
이름 입력시점에서 영어를 입력하면 [checkHasNoEnglish]에서 예외를 발생시키고 [inputMembers]라는 예외처리 메서드를 따로 구성해 올바른 문자열만 입력되게 제한
최대 짝 수를 입력하고 [getMaxGroup]를 통해 숫자 외의 다른 값이 입력되지 않게 예외처리, [checkMaxGroup]를 통해 최대 짝 수가 입력된 이름 수보다 많이 않게 예외 처리
다시 추첨하는 기능 구현을 위해 [reDraw] 메서드를 구성해 'y'가 입력되면 다시 랜덤으로 짝궁을 출력해주고, 'n'이 입력되면 프로그램을 종료함
여기서 [checkYesOrNo]를 통해 'y', 나 'n'외의 다른 문자가 입력되지 않게 예외처리

taeseokyang

수고하셨습니다!

제안 사항이 있습니다. 데이터의 유효성을 판단하는 함수에서 예외를 던지지 말고, 리턴형식을 boolean으로 하여 검사 결과를 반환하도록 바꿔보시는 걸 추천합니다.

만약 그렇게 한다면 아래와 같이 작성 할 수 있으며, try catch 문을 사용하지 않아도 됩니다!
if (!checkHasNoEnglish(members)) { break; }

예외 시 그냥 끝내는 것이 아닌 다시 진행해야 하기 때문에 위 방식이 더욱 좋을거 같습니다.

taeseokyang · 2024-04-10T11:41:09Z

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

+    }
+
+    //멤버 입력 예외처리
+    private String inputMembers(String members, Scanner scan) {


이미 run() 함수 안에서 입력받는 코드가 있는데, 이 함수 기능 또한 입력받는 함수입니다!
이렇게 중복된 코드가 생길 경우, 이후에 코드를 수정할때 두부분 모두 수정해야하는 불편함이 생깁니다.
아래와 같이 수정하면, run()함수에서 입력 코드는 없어도 될거 같아요!!

또한 'InputMismatchException'은 'nextInt()', 'nextDouble()' 메서드를 사용할때 나는 예외라고 합니다.
때문에 IllegalArgumentException() 사용하시면 될거 같습니다 :D

private String inputMembers() { Scanner scan = new Scanner(System.in); String members; while (true) { System.out.println("멤버의 이름을 한글로 다시 입력해 주세요. ',' 로 구분"); members = scan.nextLine(); try { checkHasNoEnglish(members); break; } catch (IllegalArgumentException e) { System.out.println("영어 입력 하지 마시오!"); } } return members; }

중복을 제거하는 것은 유지보수 및 가독성 향상에 도움이 되는 부분이죠👍
그런데 inputMembers를 호출할 때마다 새로운 Scanner를 생성하게 되네요. 이를 해결할 방법도 고민해보면 좋을 것 같아요^,^

taeseokyang · 2024-04-10T11:42:55Z

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

    // 문자열로된 멤버들을 리스트로 분리하는 함수입니다.
    public List<String> parseMembers(String members) {
-        return new ArrayList<>();
+        String[] arr = members.split(",");


해당 코드는 간단한 코드라 이해하기 쉽지만 변수명을 의미에 맞춰서 지으면 더욱 이해하기 좋을거 같습니다!

taeseokyang · 2024-04-10T11:43:46Z

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

+
+        //최대 짝 수 입력, 잘못된 입력 시 예외처리
+        System.out.println();
+        System.out.println("최대 짝 수를 입력해주세요.");


"\n최대 짝 수를 입력해주세요." 라고 하면 줄바꿈이 됩니다! 하지만 기존 처럼해도 오히려 명시적이어서 좋을 거 같기도 합니다!

taeseokyang · 2024-04-10T11:46:47Z

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

+    }
+
+    //최대 짝 수 입력 예외 처리
+    private static int getMaxGroup(Scanner scan) {


이 함수에 경우도 멤버 입력 함수처럼 코드가 중복됩니다! 중복되지 않도록 수정하면 좋을거 같습니다!

taeseokyang · 2024-04-10T11:49:10Z

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

    }

+    //최대 짝 수 크기 예외 처리
+    private int checkMaxGroup(List<String> memberList, int maxGroup, Scanner scan) {


마찬 가지로 중복됩니다!

System.out.println("최대 짝 수를 다시 입력해주세요");
에서 "최대 짝 수를 다시 입력해주십쇼" 라고 변경하고 싶다면 3줄의 코드를 찾아서 변경해야합니다!

taeseokyang · 2024-04-10T11:57:11Z

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

+    }
+
+    //다시 추첨 기능 + 예외처리
+    private void reDraw(List<String> memberList, int maxGroup, Scanner scan) {


다시 추첨이라는 함수명과는 다르게, 첫 생성, 다시 추첨, Y 또는 N 입력 코드등 많은 기능들이 함축되어있습니다.
하나에 함수에는 하나의 기능이 있어야 합니다( 절대적인건 아니고, 그런걸 선호합니다 )
현재 이 함수는 길고 장황하여 이해하기 어렵습니다. 모든 경우는 아니겠지만 한 함수에 최대 4줄 이라고 생각하시면 좋을거 같습니다.
때문에 들여쓰기도 3단 이상을 안넘는게 좋습니다!

때문에 각각의 한 기능을 위해 함수를 분리하여 주세요!

ay-eonii

첫 미션 고생하셨습니다!!
재입력까지 구현하신 점이 인상깊네요! 😆

ay-eonii · 2024-04-10T13:42:14Z

build.gradle

 	testImplementation platform('org.assertj:assertj-bom:3.25.1')
 	testImplementation('org.junit.jupiter:junit-jupiter')
 	testImplementation('org.assertj:assertj-core')
+	implementation("com.google.guava:guava:31.1-jre")


해당 라이브러리를 사용한 이유가 있나요?

ay-eonii · 2024-04-10T13:47:41Z

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

+    }
+
+    //멤버 입력 예외처리
+    private String inputMembers(String members, Scanner scan) {


중복을 제거하는 것은 유지보수 및 가독성 향상에 도움이 되는 부분이죠👍
그런데 inputMembers를 호출할 때마다 새로운 Scanner를 생성하게 되네요. 이를 해결할 방법도 고민해보면 좋을 것 같아요^,^

ay-eonii · 2024-04-10T13:53:02Z

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

+
+    //다시 추첨 기능 + 예외처리
+    private void reDraw(List<String> memberList, int maxGroup, Scanner scan) {
+        String index;


y 또는 n 이 index 일까요?

조금 더 의미를 담아 변수명을 지어주면 다른 사람이 코드를 이해하는데 도움이 될 것 같아요!

ay-eonii · 2024-04-10T14:19:19Z

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

    public List<String> parseMembers(String members) {
-        return new ArrayList<>();
+        String[] arr = members.split(",");
+        return new ArrayList<>(Arrays.asList(arr));


새로운 ArrayList로 감싸 반환한 이유가 있나요?
(Arrays.asList()내부를 들어가봐도 좋을 것 같아요!)

ay-eonii · 2024-04-10T14:26:52Z

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

+    private String inputMembers(String members, Scanner scan) {
+        while(true) {
+            try {
+                checkHasNoEnglish(members);


여러가지 예외도 고려해보면 재밌을 거에요~😆

ay-eonii · 2024-04-10T14:32:31Z

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

+    private void reDraw(List<String> memberList, int maxGroup, Scanner scan) {
+        String index;
+        while(true){
+            printResult(generateRandomGroups(memberList, maxGroup));


이번 미션에서 generateRandomGroups(memberList, maxGroup)이 수행하는 랜덤 그룹을 추출하는 기능이 핵심이라고 생각하는데요!
printResult의 인자로 inline하기 보다 분리한다면 어떤 부분이 핵심인지 파악하기 더 쉬울 것 같네요 😄

taeseokyang

수고하셨습니다!

hyxklee added 5 commits April 5, 2024 10:41

첫 번째 구현

9a79b41

테스트 코드 업데이트

24064cb

예외처리 로직 변경, 테스트 추가

3f84875

예외처리 메서드 생성

2ceefcf

메서드명 수정

e2b1ff7

hyxklee self-assigned this Apr 10, 2024

taeseokyang reviewed Apr 10, 2024

View reviewed changes

ay-eonii reviewed Apr 10, 2024

View reviewed changes

taeseokyang reviewed Apr 10, 2024

View reviewed changes

피드백 수정/예외처리 수정/메서드 중복 처리

eb8fabe

Comments

Conversation

hyxklee commented Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

미션 실행 결과

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

기능 명세서

Uh oh!

taeseokyang left a comment

Choose a reason for hiding this comment

Uh oh!

taeseokyang Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ay-eonii left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taeseokyang left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hyxklee commented Apr 10, 2024 •

edited

Loading

taeseokyang Apr 10, 2024 •

edited

Loading