[재연] 문자열 덧셈 계산기 #6

csbsjy · 2020-02-27T04:38:08Z

두번째 질문입니당

코드를 이렇게 수정했는데요 두가지 질문있어욤

사실 반환값이 Optional 이라 orElseThrow 를 하긴 했는데.. 실제로는 저기 null 이 있을 일이 없거든요 앞에서 다 거르기 때문에.. 그래도 저기서 던지는게 맞을까요? 처음에 .get().getValue() 하니까 좀 어색한 느낌이있어서여
reduce 사용법을 잘 몰라서 제가 이상하게 짠거같긴한데, 저렇게 짜는게 맞나여? reduce 사용하면 짧아진다구 하셨는데 어째 더 길어진거같아서염 ..!

===============================================

코드는 CalculatorServiceTest 를 시작으로 봐주시면 좋을 것 같습니다.

한가지 고민인점은, Splitter 클래스에서

위 클래스변수에 대해 어떻게 생각하시는지요?

static final 에는 기본형, 불변클래스를 사용하라고 이펙티브자바에서 본 것 같은데 패턴 클래스도 까보니까 스레드세이프한 애 같아서 했거든여.

해도 문제 없다면 위와 같은 변수는 상수가 아니니까 대문자와 _ 를 섞어쓰는 컨벤션을 따르지 않아도 되는걸까욤?

final 이 붙으면 상수. Pattern 도 상수가 된 것이기 때문에 대문자 _ 컨벤션을 따라야한다.

1. Calculator, CalculatorService, Splitter로 책임 분리 2. TestCode 작성 완료

1. Split Class Matcher 로 로직 변경 2. 테스트코드 변경 3. Pattern 처럼 스레드세이프한 객체의 상수화. 네이밍은 대문자? 소문자?

1. 서비스 불필요한 validation 제거 및 테스트 수정

src/main/java/domain/Calculator.java

1. Double > Long 안바뀌어있던 것 수정

src/main/java/support/Splitter.java

src/main/java/domain/dto/Operand.java

chldbtjd2272 · 2020-02-28T15:48:33Z

src/main/java/domain/Calculator.java

+public class Calculator {
+    public long calculate(List<String> operands) {
+        return operands.stream()
+                .map(Operand::new)


reduce를 사용하면 한줄로 가능할거 같고 Operand의 값을 꺼내서 하는것보단 새로운 두개의 값을 더해서 새로운 오퍼랜드를 꺼내는게 좋을듯

선생님 reduce 너무 어려워요

src/test/java/domain/CalculatorTest.java

src/test/java/service/CalculatorServiceTest.java

src/test/java/support/SplitterTest.java

chldbtjd2272 · 2020-02-28T16:04:13Z

src/main/java/service/CalculatorService.java

+        this.calculator = calculator;
+    }
+
+    public long calculate(final String formula) {


요구사항 1번 빈문자열은 0리턴, 2번 숫자 하나를 입력받으면 해당숫자반환은 없는거같은데,
위와같이 요구사항이 명확한 경우에는 테스트 코드로 해당 상황들을 먼저 정의하고 실제 구현을 하는게 좋을듯 (TDD)

2. Calculator logic reduce로 변경 -> 질문 3. testcode 수정 Operand::sum 오타수정 Pattern convention

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

src/main/java/service/CalculatorService.java

+        }
+        List<String> splitedFormula = Splitter.split(formula.trim());
+
+        return calculator.calculate(splitedFormula);


Splitter.split(formula.trim()) 자체가 splited formula 라는 의미를 담고있는데, 변수명을 굳이 써야할까..?!!
아래 리턴에 같이 합쳐도 될것같음

오호 그러넹
혹시 가독성 떨어질줄알았는데 바꾸고보니까 생각보다 깔끔하넹 ㅎㅅㅎ 반영했오 땡큐 !

pci2676 · 2020-03-04T03:44:02Z

src/main/java/domain/Calculator.java

+                .reduce(Operand::sum)
+                .orElseThrow(() -> new IllegalArgumentException("sum 할 수 있는 operand가 없습니다"))


reduce할때 첫번째 인자로 0을 주면 Optional한 결과가 나오지 않아!

Integer sum = integers.reduce(0, Integer::sum);

선생님 제 코드는 지금 Operand 를 반환하눈데용, 그럼 혹시 new Operand() 를 해서 0 을 가지고 있는 애를 반환하는게 좋을까? 지금은 디폴드생성자가 없는 상태이긴행 1

하긴 어차피 Null 일 일이 없으니까 한줄로 끝내는게 나아보이긴한다!

pci2676 · 2020-03-04T03:45:56Z

src/main/java/domain/dto/Operand.java

@@ -0,0 +1,49 @@
+package domain.dto;
+
+public class Operand {


ㅎㅎ 이왕 VO로 만든거 final로 만들어 주는건 어떨까!
상속받을일도 없을거 같기도 하고..!!
근데 나도 안했던거같아..

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ

pci2676 · 2020-03-04T03:47:55Z

src/main/java/domain/dto/Operand.java

+        return new Operand(this.value + operand.value);
+    }
+
+    private void checkNegative(long parsedOperand) {


private 메서드는 그와 관련있는 public 메서드 근처에 위치해서 짝을 이루는게 읽기 좋다고 생각해!
생성자에서 쓰이는 메서드니까 그 아래에 있으면 난 좋을거 같아

pci2676 · 2020-03-04T03:52:34Z

src/main/java/support/Splitter.java

+        if (customDelimiter.matches(NUMERIC_REGEX) || customDelimiter.isEmpty()) {
+            throw new IllegalArgumentException(String.format("%s 는(은) 올바르지 않은 커스텀구분자입니다.", customDelimiter));
+        }


헉 구분자가 숫자가 되는 경우에 대해서도 유효성을 잡아줬구나
난 생각치도 못했어..!

2. final static -> static final

malibinYun · 2020-03-06T04:55:48Z

src/main/java/service/CalculatorService.java

+            return 0;
+        }
+
+        return calculator.calculate(Splitter.split(formula.trim()));


이 구문은 한줄에 .이 너무 많아서 가독성이 안좋아보여요.
Splitter.split(formula.trim()) 이 친구를 한 번 래핑하는 편이 더 나아보입니다.

오홍 의견이갈리네여 ~~ 아래 의견도 있었었습니당 ㅋㅋㅋ
다시 고민해볼게여!

csbsjy added 4 commits February 26, 2020 16:46

1st commit

b10cf9f

1. Calculator, CalculatorService, Splitter로 책임 분리 2. TestCode 작성 완료

[2nd Commit]

7ec8ac0

1. Split Class Matcher 로 로직 변경 2. 테스트코드 변경 3. Pattern 처럼 스레드세이프한 객체의 상수화. 네이밍은 대문자? 소문자?

[3rd commit]

f9e658b

1. 서비스 불필요한 validation 제거 및 테스트 수정

codeconvention

59af198

csbsjy requested review from Gobukgol, Gyeom, bominjang, chldbtjd2272, dongry1002, jyami-kim, malibinYun and pci2676 February 27, 2020 04:38

jyami-kim reviewed Feb 28, 2020

View reviewed changes

src/main/java/domain/Calculator.java Outdated Show resolved Hide resolved

Codeconvention, double > long 수정

55d923c

1. Double > Long 안바뀌어있던 것 수정

chldbtjd2272 reviewed Feb 28, 2020

View reviewed changes

csbsjy added 5 commits February 29, 2020 22:39

1. 요구사항 추가

e3309b5

2. Calculator logic reduce로 변경 -> 질문 3. testcode 수정 Operand::sum 오타수정 Pattern convention

Operand 생성자 private

7cbb28e

Operand Test 코드 생성

e22bf1e

SplitTest 수정

10cab20

정적 Util 클래스는 기본생성자를 private으로 막자

54ae35e

jyami-kim reviewed Mar 3, 2020

View reviewed changes

Splitter.split 을 변수로 빼지 않아보자

d146923

pci2676 reviewed Mar 4, 2020

View reviewed changes

csbsjy added 2 commits March 4, 2020 14:26

1. Operand 기본생성자 추가, reduce 수정

6070a64

2. final static -> static final

@NullAndEmptySource 를 알았다

dfcaa89

malibinYun reviewed Mar 6, 2020

View reviewed changes

		.reduce(Operand::sum)
		.orElseThrow(() -> new IllegalArgumentException("sum 할 수 있는 operand가 없습니다"))

[재연] 문자열 덧셈 계산기 #6

Are you sure you want to change the base?

[재연] 문자열 덧셈 계산기 #6

Uh oh!

Conversation

csbsjy commented Feb 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

csbsjy commented Feb 27, 2020 •

edited

Loading

csbsjy Mar 4, 2020 •

edited

Loading