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

문자열 계산기 구현했습니다! #2

Open
wants to merge 6 commits into
base: chae-heechan
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
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,16 @@ String[] values = value.split(" ");
- 문자열을 숫자로 변경하는 방법

`int number = Integer.parseInt("문자열");`

### 기능
- 예외
- [x] 숫자로 시작하지 않을 경우
- [x] 띄어쓰기가 잘 안 돼서 숫자와 문자가 섞였을 경우(공백문자 미사용)
- [x] 숫자와 연산자 이외의 문자
- [x] 연산자나 숫자가 두 번 이상 연속으로 나올 경우
- 순서
- [x] 문자열 입력 받기
- [x] 띄어쓰기로 분리해서 배열에 넣기
- [x] 예외 처리
- [x] 피연산자(operand 숫자)와 연산자(operator +-/*)로 나누기
- [x] 계산하기
Comment on lines +23 to +34

Choose a reason for hiding this comment

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

예외 케이스까지 잘 적어주셨네요 👍🏻

17 changes: 17 additions & 0 deletions src/main/java/Application.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import java.util.List;
import java.util.Scanner;

public class Application {
public static void main(String[] args) {
Scanner scanner = new Scanner(System.in);
String input = scanner.nextLine();
Comment on lines +6 to +7

Choose a reason for hiding this comment

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

비즈니스 로직과 뷰 로직의 분리 👍🏻
근데 애플리케이션이 너무 불친절한것같아요!
안내메시지 없이 밑도끝도 없이 숫자를 입력하라그러네요 😂

Comment on lines +6 to +7

Choose a reason for hiding this comment

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

Application의 역할은 무엇일까요?
그리고 입력해야 하는 메소드와 출력해야 하는 메소드가 많아질 경우 어떻게 해야할까요?


Operation operation = new Operation(input);
List<Integer> operands = operation.getOperands();
List<Character> operators = operation.getOperators();

Calculator calculator = new Calculator();
double result = calculator.calculate(operands, operators);
System.out.println("result = " + result);
}
}
41 changes: 41 additions & 0 deletions src/main/java/Calculator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

import java.util.List;

public class Calculator {

public double calculate(List<Integer> operands, List<Character> operators) {
double result = operands.get(0);
int numOfOperations = operators.size();
int times = 0;

while (times < numOfOperations) {
result = fourFundamentalArithmeticOperations(result, operators.get(times), operands.get(++times));
}

return result;
}

public double fourFundamentalArithmeticOperations(double tempResult, char operator, double operand) {
double result;

switch (operator) {
case '+':
result = tempResult + operand;
break;
case '-':
result = tempResult - operand;
break;
case '*':
result = tempResult * operand;
break;
case '/':
result = tempResult / operand;
break;
Comment on lines +31 to +33

Choose a reason for hiding this comment

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

0으로 나눌 때의 에러도 함께 있으면 좋을거 같아요!

default:
result = 0;
break;
Comment on lines +21 to +36

Choose a reason for hiding this comment

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

계산기에서 지원하는 연산자가 증가할때마다 이 switch문이 비례해서 증가할 것 같아요.
어떻게 하면 이것을 해결할 수 있을까요?
인터페이스와 다형성을 학습해보세요.

}

return result;
}
}
65 changes: 65 additions & 0 deletions src/main/java/Operation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@

import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class Operation {
private final String[] operation;

Choose a reason for hiding this comment

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

쪼갠 스트링 배열을 굳이 상태(필드)로 갖고 있을 필요가 있을까요?
클래스의 필드가 많아지면 무엇이 단점일까요?

private List<Integer> operands;
private List<Character> operators;

public Operation(String input) {
this.operation = input.split(" ");
validateOperation();
splitOperation();
}
Comment on lines +12 to +16

Choose a reason for hiding this comment

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

Operation�의 역할이 너무 많은것같아요.
Operation이 지금 어떤 행위들을 하고있나요?
SRP에 대해 학습해보세요.


public void validateOperation() {
validateOtherSymbols();
validateFirstIndex();
validateDuplicate();
Comment on lines +19 to +21

Choose a reason for hiding this comment

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

상세한 예외 검증 👍🏻

}

public void validateOtherSymbols() {
for (String tempStr : operation) {
if (!(tempStr.matches("^[0-9]*$") || tempStr.matches("^\\-[1-9]\\d*$") || tempStr.matches("[+*/-]")))
throw new IllegalArgumentException("한 String에 숫자와 연산자가 함께 있거나, 숫자 연산자 이외의 입력입니다.");

Choose a reason for hiding this comment

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

예외 메시지 👍🏻

}
}

public void validateFirstIndex() {
if (operation[0].matches("[+*/-]")) {
throw new IllegalArgumentException("숫자로 시작해야 합니다.");
}
}
Comment on lines +31 to +35

Choose a reason for hiding this comment

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

이 예외는 언제 발생하나요? validateOtherSymbols에서 다 검증이 가능할것같아요.


public void validateDuplicate() {
for (int i = 0; i < operation.length - 1; i++) {
if ((!operation[i].matches("[+*/-]") && !operation[i + 1].matches("[+*/-]"))
|| (operation[i].matches("[+*/-]") && operation[i + 1].matches("[+*/-]"))) {
Comment on lines +39 to +40

Choose a reason for hiding this comment

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

처음 투입한 프로젝트에서 이 부분만 보고 무엇을 의미하는지 알 수 있을까요?
boolean을 리턴하는 메소드로 분리하는건 어떨까요?

throw new IllegalArgumentException("기호나 숫자가 두 번 연속 입력되었습니다.");
}
}
}

public void splitOperation() {
operands = Arrays.stream(this.operation)
.filter(operand -> operand.matches("^[0-9]*$") || operand.matches("^\\-[1-9]\\d*$"))
.map(Integer::parseInt)
.collect(Collectors.toList());

operators = Arrays.stream(this.operation)
.filter(operator -> operator.matches("[+*/-]"))
.map(operator -> operator.charAt(0))
.collect(Collectors.toList());
Comment on lines +47 to +55

Choose a reason for hiding this comment

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

Operation에서 operands와 operators를 모두 처리하려다 보니 Operation의 책임도 커지고 로직도 길어지는 경향이 생기는 것 같아요.
일급 컬렉션을 사용해 보는건 어떨까요?
이 부분은 너무 어렵게 느껴지시면 반영 안하셔도 됩니다.

}

public List<Integer> getOperands() {
return operands;
}

public List<Character> getOperators() {
return operators;
}
}
43 changes: 43 additions & 0 deletions src/test/java/CalculatorTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@

import static org.assertj.core.api.Assertions.*;

import java.util.Arrays;
import java.util.List;

import org.junit.jupiter.api.Test;

class CalculatorTest {

private List<Integer> operands = Arrays.asList(2, 4, 3, 7, 5);
private List<Character> operators = Arrays.asList('+', '-', '*', '/');

private Calculator calculator = new Calculator();

@Test
void calculate() {
double result;

result = calculator.calculate(operands, operators);

assertThat(result).isEqualTo(4.2);
}

@Test
void 사칙연산() {
double addition, subtraction, multiplication, division, wrongOperatorResult;

addition = calculator.fourFundamentalArithmeticOperations(operands.get(0), operators.get(0), operands.get(1));
subtraction = calculator.fourFundamentalArithmeticOperations(operands.get(0), operators.get(1),
operands.get(1));
multiplication = calculator.fourFundamentalArithmeticOperations(operands.get(0), operators.get(2),
operands.get(1));
division = calculator.fourFundamentalArithmeticOperations(operands.get(0), operators.get(3), operands.get(1));
wrongOperatorResult = calculator.fourFundamentalArithmeticOperations(operands.get(0), '?', operands.get(1));

assertThat(addition).isEqualTo(6.0);
assertThat(subtraction).isEqualTo(-2.0);
assertThat(multiplication).isEqualTo(8.0);
assertThat(division).isEqualTo(0.5);
assertThat(wrongOperatorResult).isEqualTo(0);
}
Comment on lines +25 to +42

Choose a reason for hiding this comment

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

더하기, 빼기, 곱하기, 나누기 등 기능 별로 따로 테스트를 만드는 건 어떨까요!!
예를 들어 이 테스트에서 뺄셈에서만 작동하지 않을 경우에도 이 테스트 전체에서 작동하지 않는다고 나올거 같아서요!

Choose a reason for hiding this comment

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

동의합니다!
단위 테스트이기 때문에 최대한 작은 단위를 테스트 해주시는것이 좋아요.
또한 테스트도 유지보수의 대상이기 때문에 작게 유지할 수록 좋답니다.

}
64 changes: 64 additions & 0 deletions src/test/java/OperationTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@

import static org.assertj.core.api.Assertions.*;

import java.util.ArrayList;
import java.util.Arrays;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class OperationTest {

private Operation operation;

@BeforeEach
void setUp() {
operation = null;
}

@Test
void validateMixed() {
String wrongInput = "2/ 3";

assertThatThrownBy(() -> operation = new Operation(wrongInput))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("한 String에 숫자와 연산자가 함께 있거나, ");
Comment on lines +23 to +25

Choose a reason for hiding this comment

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

assertJ의 다양한 메소드를 잘 활용해주셨네요!
현재 모든 예외를 IllegalArgumentException으로 처리하고있어서, 메시지를 통해 확인한 점도 좋습니다 👍🏻

}

@Test
void validateOtherSymbols() {
String wrongInput = "( 3 - 4";

assertThatThrownBy(() -> operation = new Operation(wrongInput))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("숫자 연산자 이외의 입력입니다");
}

@Test
void validateFirstIndex() {
String wrongInput = "- 2 + 1";

assertThatThrownBy(() -> operation = new Operation(wrongInput))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("숫자로 시작해야 합니다.");
}

@Test
void validateDuplicate() {
String wrongInput = "2 * / 1 + 3";

assertThatThrownBy(() -> operation = new Operation(wrongInput))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("기호나 숫자가 두 번 연속");
}

@Test
void splitOperation() {
String input = "2 + -3 * 4 / 2";

operation = new Operation(input);

assertThat(operation.getOperands()).isEqualTo(Arrays.asList(2, -3, 4, 2));
assertThat(operation.getOperators()).isEqualTo(Arrays.asList('+', '*', '/'));
}
}