-
Notifications
You must be signed in to change notification settings - Fork 6
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
계산기 구현 #1
base: Jummi10
Are you sure you want to change the base?
계산기 구현 #1
Changes from all commits
cde694a
b8146b8
858f32d
8949088
30cb7c4
b6e8aab
2049377
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import java.util.List; | ||
import java.util.Scanner; | ||
|
||
import calculator.Calculator; | ||
import calculator.Operation; | ||
|
||
public class Application { | ||
public static void main(String[] args) { | ||
Scanner scanner = new Scanner(System.in); | ||
String input = scanner.nextLine(); | ||
|
||
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); | ||
Comment on lines
+9
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. main에서 프로그램의 흐름이 느껴지네요 👍 |
||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,42 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package calculator; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+8
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. times라는 변수명:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public double fourFundamentalArithmeticOperations(double tempResult, char operator, double operand) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메서드 이름:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
double result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
switch (operator) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. switch문은 동민님이 말했듯, 선호되지 않는 방법입니다. enum을 활용하여 리팩토링 해보는것도 좋을것 같아요 🙂 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case '+': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result = tempResult + operand; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case '-': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result = tempResult - operand; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case '*': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result = tempResult * operand; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case '/': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result = tempResult / operand; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
default: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
result = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
break; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+19
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. switch 문을 활용해 결과를 깔끔하게 처리하셨군요! 👍
Suggested change
한 가지 덧붙이자면 보통 switch 문은 함수의 길이를 지나치게 길게 만들기 때문에 기피하고는 한답니다. (클린코드, 47p) 하지만 switch 문 자체를 완전히 배제할 수는 없기 때문에 책에서 제안하는 방법은 다형성을 활용해서 switch 문을 풀어내는 것입니다. 자세한 내용은 책을 참조하시면 좋을 것 같습니다! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package calculator; | ||
|
||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
|
||
public class Operation { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 개인적으로 Operation 클래스의 책임이 너무 많다고 느껴집니다.
클래스를 추가하여 책임을 나누는 것이 어떨지 제안해봅니다 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 객체는 독자적인 데이터(상태)를 갖고, 갖고 있는 데이터를 이용하여 행동을 합니다.
객체지향의 사실과 오해(2장) 정리
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 모든 메소드가 public으로 정의되어 있네요. 괜찮을까요? |
||
private final String[] operation; | ||
private List<Integer> operands; | ||
private List<Character> operators; | ||
|
||
public Operation(String input) { | ||
this.operation = input.split(" "); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
validateOperation(); | ||
splitOperation(); | ||
} | ||
|
||
public void validateOperation() { | ||
validateOtherSymbols(); | ||
validateFirstIndex(); | ||
validateDuplicate(); | ||
} | ||
|
||
public void validateOtherSymbols() { | ||
for (String tempStr : operation) { | ||
if (!(tempStr.matches("^[0-9]*$") || tempStr.matches("^\\-[1-9]\\d*$") || tempStr.matches("[+*/-]"))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "[]"로 묶어서 사칙 연산 검증을 할 수도 있군요. 더 깔끔해보입니다 👍 하지만 정규표현식이 Operation 클래스의 여러 메서드에 반복적으로 등장하는 만큼 이름에 의미가 부여된 상수나 메서드를 활용하시면 더 읽기 좋은 코드가 될 것 같습니다! 한 가지 궁금한게 있는데 숫자를 검증하는 정규 표현식이 *라면 비어있는 문자열도 통과시켜 버그가 나지 않나 조심스레 질문 남깁니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오호
이므로 |
||
throw new IllegalArgumentException("한 String에 숫자와 연산자가 함께 있거나, 숫자 연산자 이외의 입력입니다."); | ||
} | ||
} | ||
|
||
public void validateFirstIndex() { | ||
if (operation[0].matches("[+*/-]")) { | ||
throw new IllegalArgumentException("숫자로 시작해야 합니다."); | ||
} | ||
} | ||
|
||
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("[+*/-]"))) { | ||
throw new IllegalArgumentException("기호나 숫자가 두 번 연속 입력되었습니다."); | ||
} | ||
Comment on lines
+38
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. regex의 활용은 좋습니다 👍 |
||
} | ||
} | ||
|
||
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()); | ||
} | ||
|
||
public List<Integer> getOperands() { | ||
return operands; | ||
} | ||
|
||
public List<Character> getOperators() { | ||
return operators; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package calculator; | ||
|
||
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); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package calculator; | ||
|
||
import static org.assertj.core.api.Assertions.*; | ||
|
||
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. null로 초기화 하는것은 setUp 메소드를 제대로 활용한 것 같진 않네요. 모든 테스트에서 매번 인스턴스를 초기화를 하고 있는 불편함이 해결되지 않은 느낌이에요. |
||
} | ||
|
||
@Test | ||
void validateMixed() { | ||
String wrongInput = "2/ 3"; | ||
|
||
assertThatThrownBy(() -> operation = new Operation(wrongInput)) | ||
.isInstanceOf(IllegalArgumentException.class) | ||
.hasMessageContaining("한 String에 숫자와 연산자가 함께 있거나, "); | ||
} | ||
|
||
@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('+', '*', '/')); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
연산자의 클래스가 Character로 사용되는건 마음이 아프네요 😥
연산자 객체를 따로 만들수 있진 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Character로 사용되는게 왜 마음이 아픈지 알 수 있을까요.. 직접 정의한 객체로 분리하라는 말은 first-class 컬렉션을 사용해보라는 말씀이신가요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Character 클래스는 너무 광범위하지 않나요?
Charactor 클래스가 연산을 처리하기에 적절한 클래스일까요?
일급컬렉션을 사용해보라는 뜻은 아니었습니다. (물론 적용이 가능한 지점이긴 합니다 🙂)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calculator가 Character 리스트를 확인하고 그에 맞는 연산을 하고 있습니다.
Calculator 객체의 책임이 버겁진 않나요? (단일 책임 원칙)
객체간의 협력을 통해 책임을 나누어 해결할 수 있지 않을까요?