[정재]String-adding-calculation #7

dongry1002 · 2020-03-01T11:06:17Z

CustomerDelimeter 추가예정
배열이 아니라 ArrayList로 바꾸자

jyami-kim · 2020-03-03T15:32:03Z

src/main/java/calculator/Calclator.java

+package calculator;
+
+public class Calclator {
+    Spliter spliter = new Spliter();


여기서 선언한 Spliter가 새로 바뀌거나 한다면 위처럼 가변 참조 필드로 두어도 되겠지만, 그것이 아니기 때문에 private final 걸어주면 좋을것같아요

혹시 private final로 걸어야하는부분이 spliter 인가요?ㅠ

src/main/java/calculator/Calclator.java

jyami-kim · 2020-03-03T15:35:45Z

src/main/java/calculator/CalculatorApplication.java

+        int result = 0;
+        System.out.println("숫자들을 입력하세요 : ");
+        Scanner scanner = new Scanner(System.in);
+        String inputValue = scanner.nextLine();
+        if (inputValue.isEmpty()) {
+            System.out.println(result);
+            return;
+        }
+
+        Spliter spliter = new Spliter();
+        String[] splitedValue = spliter.splitValue(inputValue);
+
+        System.out.println(splitedValue);
+        Calclator calclator = new Calclator();
+        result = calclator.sum(splitedValue);
+
+        System.out.println(result);


result라는 변수가 꼭 필요한가 싶습니다.

Suggested change

int result = 0;

System.out.println("숫자들을 입력하세요 : ");

Scanner scanner = new Scanner(System.in);

String inputValue = scanner.nextLine();

if (inputValue.isEmpty()) {

System.out.println(result);

return;

}

Spliter spliter = new Spliter();

String[] splitedValue = spliter.splitValue(inputValue);

System.out.println(splitedValue);

Calclator calclator = new Calclator();

result = calclator.sum(splitedValue);

System.out.println(result);

System.out.println("숫자들을 입력하세요 : ");

Scanner scanner = new Scanner(System.in);

String inputValue = scanner.nextLine();

if (inputValue.isEmpty()) {

System.out.println(0);

return;

}

Spliter spliter = new Spliter();

String[] splitedValue = spliter.splitValue(inputValue);

System.out.println(splitedValue);

Calclator calclator = new Calclator();

System.out.println(calclator.sum(splitedValue));

jyami-kim · 2020-03-03T15:37:25Z

src/main/java/calculator/Spliter.java

+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public class Spliter {


https://88240.tistory.com/440
참고하세요 :)

가변필드를 가지고 있지 않고 Util 성을 띄고있는 것 같아 static 클래스로 만들어줘도 좋을 것 같습니다 ! 그렇게 하면 Main과 Calculator 에서 불필요하게 두번씩 생성할 필요도 없을 것 같습니다!

네 static으로 바꿨습니다. 감사합니다~

jyami-kim · 2020-03-03T15:38:03Z

src/main/java/calculator/Spliter.java

+
+    public void checkPositve(int parseInt) {
+        if (parseInt > 0) {
+            return;


여기있는 return 부분이 불필요해 보입니다. if 안의 조건문을 변경해보세요

네 변경하였습니다.

jyami-kim · 2020-03-03T15:39:35Z

src/main/java/calculator/Spliter.java

+        System.out.println(splitedValue[0]);
+        System.out.println(splitedValue[1]);
+        System.out.println(splitedValue[2]);
+        return splitedValue;


이렇게 인덱싱을 했을때 outofbound 에러가 나지는 않을지 예외케이스를 생각해보세요

jyami-kim · 2020-03-03T15:41:03Z

src/main/java/calculator/Spliter.java

+    }
+
+    public int parseToInt(String splitedValue) {
+        if (isNumberic(splitedValue)) {


isNumberic과 아래 코드에서 parseInt를 두번 체크하는데 다른 방식을 고려해보세요

csbsjy · 2020-03-04T00:01:52Z

src/main/java/calculator/Spliter.java

+            checkPositve(number);
+            return number;
+        }
+        throw new IllegalArgumentException("양수로 된 숫자만 입력하세요.");


isNumeric 의 catch 문에서 throw 하는 것은 어떨까요?? return false를 굳이 하지 않아도 될 것 같아요 ! isNumeric ParseInt 중복 부분을 해결하면 같이 해결될 것 같습니다 ~

csbsjy · 2020-03-04T00:52:16Z

src/main/java/calculator/Spliter.java

+    public static final String CUSTOMER_DELIMITER = "//(.)\n(.*)";
+
+    public String[] splitValue(String value) {
+        Matcher m = Pattern.compile(CUSTOMER_DELIMITER).matcher(value);


여기서 Pattern.complie 은 항상 같은 Pattern 을 가리키잖아요?
Pattern 은 생성 비용이 높기 때문에 상수로 초기화시키고 사용하는 것이 좋다고 하더라구요!

Suggested change

Matcher m = Pattern.compile(CUSTOMER_DELIMITER).matcher(value);

private static final Pattern CUSTOM_PATTERN = Pattern.compile(CUSTOMER_DELIMITER);

아, 그리고 이 클래스의 멤버변수는 private 이어도 될 것 같습니다! Splitter 클래스만 알고 있어도 되는 변수로 보여요!

https://javabom.tistory.com/30

csbsjy · 2020-03-04T00:54:01Z

src/main/java/calculator/Spliter.java

+            String customDelimiter = m.group(1);
+            String tokens =m.group(2);
+            String[] splitedValue = tokens.split(CUSTOMER_DELIMITER);
+            System.out.println(splitedValue[0]+"custom");


System.out 은 테스트용인건가요!?

넵 지웠습니다. 감사해요

pci2676 · 2020-03-04T04:06:01Z

src/main/java/calculator/Spliter.java

+    public static final String DELIMITER1 = "[,|:]";
+    public static final String CUSTOMER_DELIMITER = "//(.)\n(.*)";


해당 상수들이 public하게 공개되어있을 필요가 없을것 같아여!

pci2676 · 2020-03-04T04:09:01Z

src/main/java/calculator/Spliter.java

+    public static final String CUSTOMER_DELIMITER = "//(.)\n(.*)";
+
+    public String[] splitValue(String value) {
+        Matcher m = Pattern.compile(CUSTOMER_DELIMITER).matcher(value);


Matcher 객체의 변수를 m으로 사용하고 있네요!
축약은 코드를 읽는 사람들에게 혼동을 줄 수 있어요!

pci2676 · 2020-03-04T04:11:39Z

src/test/java/calculator/SpliterTest.java

+    @Test
+    void splitValue() {
+//        String inputValue = new String("1,2:3");
+//        assertThat(spliter.splitValue(inputValue)).isEqualTo(["1", "2", "3"]);
+    }


불필요한 테스트는 제거해주세요 ㅎㅎ

추가적으로 기본식이 입력 되었을 때와 커스텀 구분자가 들어간 식에 대한 계산 테스트가 존재하지 않네요!
만약 해당 테스트를 작성하기 힘들다면 작성하게 힘들어지는 부분을 찾고 리팩토링을 해보는게 좋을것 같아요!
참고로 커스텀 구분자는 \n때문에 테스트 작성하기가 힘들어 질수도 있어요 이때는 @ParameterizedTest와 @MethodSource를 이용해 보면 좋을것 같아요!

pci2676 · 2020-03-04T04:21:59Z

src/main/java/calculator/Spliter.java

+        try {
+            Integer.parseInt(splitedValue);
+            return true;
+        } catch (IllegalArgumentException e) {
+            return false;
+        }


이펙티브자 자바3판 아이템 69 예외는 진짜 예외 상황에만 사용하라

한번 검색해서 참고해 보시면 좋을것 같아요!
저는 지금 이 코드가 제어 흐름용 exception 이라고 생각해요!

pci2676 · 2020-03-04T04:25:10Z

src/main/java/calculator/CalculatorApplication.java

+    public static void main(String[] args) {
+        int result = 0;
+        System.out.println("숫자들을 입력하세요 : ");
+        Scanner scanner = new Scanner(System.in);
+        String inputValue = scanner.nextLine();
+        if (inputValue.isEmpty()) {
+            System.out.println(result);
+            return;
+        }
+
+        Spliter spliter = new Spliter();
+        String[] splitedValue = spliter.splitValue(inputValue);
+
+        System.out.println(splitedValue);
+        Calclator calclator = new Calclator();
+        result = calclator.sum(splitedValue);
+
+        System.out.println(result);


main 메서드길이를 최대 10라인이 되도록 수정해주세요!
저희 이번 미션 프로그래밍 요구사항에 포함되어 있었습니다 ㅎㅎ

dongry1002 added 5 commits February 29, 2020 23:15

프로덕트코드 구현 ,테스트코드구현해야함

c661cae

프로덕트코드 ,테스트코드작성해야함

2fae56d

customerDelimiter추가예정

109fc3a

customerDelimiter추가예정

11fd829

완성

1e3a8c4

jyami-kim reviewed Mar 3, 2020

View reviewed changes

csbsjy reviewed Mar 4, 2020

View reviewed changes

pci2676 requested changes Mar 4, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[정재]String-adding-calculation #7

[정재]String-adding-calculation #7

dongry1002 commented Mar 1, 2020

jyami-kim Mar 3, 2020

dongry1002 Mar 8, 2020

jyami-kim Mar 3, 2020

jyami-kim Mar 3, 2020

csbsjy Mar 4, 2020 •

edited

Loading

dongry1002 Mar 8, 2020

jyami-kim Mar 3, 2020

dongry1002 Mar 8, 2020

jyami-kim Mar 3, 2020

jyami-kim Mar 3, 2020

csbsjy Mar 4, 2020 •

edited

Loading

csbsjy Mar 4, 2020

csbsjy Mar 4, 2020

csbsjy Mar 4, 2020

dongry1002 Mar 8, 2020

pci2676 Mar 4, 2020

pci2676 Mar 4, 2020

pci2676 Mar 4, 2020

pci2676 Mar 4, 2020

pci2676 Mar 4, 2020

pci2676 Mar 4, 2020

	Matcher m = Pattern.compile(CUSTOMER_DELIMITER).matcher(value);
	private static final Pattern CUSTOM_PATTERN = Pattern.compile(CUSTOMER_DELIMITER);

		public static final String DELIMITER1 = "[,\|:]";
		public static final String CUSTOMER_DELIMITER = "//(.)\n(.*)";

[정재]String-adding-calculation #7

Are you sure you want to change the base?

[정재]String-adding-calculation #7

Conversation

dongry1002 commented Mar 1, 2020

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

csbsjy Mar 4, 2020 • 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

csbsjy Mar 4, 2020 • 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

csbsjy Mar 4, 2020 •

edited

Loading

csbsjy Mar 4, 2020 •

edited

Loading