Skip to content

[4기 -유원우] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #167

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

Open
wants to merge 16 commits into
base: wonu606
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
6fa2360
feat: 조회와 계산 기능을 담을 인터페이스 구현
wonu606 Jun 11, 2023
490ce1d
test: 저장소 구현체 테스트 작성
wonu606 Jun 11, 2023
728fcad
feat: 저장소 구현체, 메시지를 담을 클래스 구현
wonu606 Jun 11, 2023
b99177b
refactor: Strategy 인터페이스 -> CalculatorStrategy 로 이름 변경
wonu606 Jun 11, 2023
bac3c9f
refactor: CalculatorApp 객체에서 Input, Print 필드 제거 및 함수화
wonu606 Jun 11, 2023
463f199
refactor: 입력시 throw IOException 예외 명시 삭제
wonu606 Jun 12, 2023
8a83609
feat: 연산자 구현
wonu606 Jun 12, 2023
72db880
feat: 중위 연산식 유효 검사 클래스 구현
wonu606 Jun 12, 2023
8f347a2
test: 중위 연산식 유효 검사 클래스 테스트 작성 및 통과
wonu606 Jun 12, 2023
95306f6
feat: 중위 연산식을 후위 연산식으로 바꾸는 converter 구현
wonu606 Jun 14, 2023
400b2a4
test: 중위 연산식을 후위 연산식으로 변환하는 Converter 테스트 작성 및 통과
wonu606 Jun 14, 2023
60a1eb4
feat: 후위 연산식을 계산하는 클래스 구현
wonu606 Jun 14, 2023
3dfc78e
test: 후위 연산식을 계산하는 클래스 테스트 작성 및 통과
wonu606 Jun 14, 2023
ef0ddd8
chore: 불필요한 import문 제거 및 디렉토리 재정렬
wonu606 Jun 14, 2023
52ddf84
test: 중위 연산식을 입력 받아 계산하는 클래스 테스트 작성
wonu606 Jun 14, 2023
7bba45d
feat: 계산하는 구현체 구현 및 테스트 통과
wonu606 Jun 15, 2023
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
5 changes: 2 additions & 3 deletions calculator/src/main/java/com/wonu606/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@

import com.wonu606.app.App;
import com.wonu606.calculator.CalculatorApp;
import com.wonu606.io.ConsoleInput;
import com.wonu606.io.ConsolePrinter;
import com.wonu606.io.Input;
import com.wonu606.io.Print;
import com.wonu606.io.ConsoleInput;
import java.io.IOException;

public class Main {

Expand All @@ -25,7 +24,7 @@ public static void main(String[] args) {
}
}

private static void runApp(App app, Input input, Print printer) throws IOException {
private static void runApp(App app, Input input, Print printer) {
app.execute(input, printer);
}
}
3 changes: 1 addition & 2 deletions calculator/src/main/java/com/wonu606/app/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

import com.wonu606.io.Input;
import com.wonu606.io.Print;
import java.io.IOException;

public interface App {

void execute(Input input, Print printer) throws IOException;
void execute(Input input, Print printer);
}
48 changes: 41 additions & 7 deletions calculator/src/main/java/com/wonu606/calculator/CalculatorApp.java
Original file line number Diff line number Diff line change
@@ -1,21 +1,55 @@
package com.wonu606.calculator;

import com.wonu606.app.App;
import com.wonu606.calculator.calculator.PostfixCalculator;
import com.wonu606.calculator.converter.InfixToPostfixConverter;
import com.wonu606.calculator.storage.Persistence;
import com.wonu606.calculator.storage.ResultStore;
import com.wonu606.calculator.strategy.CalculationStrategy;
import com.wonu606.calculator.strategy.CalculatorStrategy;
import com.wonu606.calculator.util.CalculatorMessage;
import com.wonu606.calculator.validator.InfixValidator;
import com.wonu606.io.Input;
import com.wonu606.io.Print;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

public class CalculatorApp implements App {

Input input;
Print printer;
private final Map<String, CalculatorStrategy> strategies = new HashMap();
Copy link

Choose a reason for hiding this comment

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

Map으로 CalculatorStrategy를 관리하는 것의 장점이 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

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

메뉴 번호와 기능 자체를 한 곳에서 모아두고 관리하고 싶어 사용하였습니다.
만약 메뉴 번호도 추가되고 기능 자체도 계산기앱에 추가될 경우 Map.put()만으로 추가하는 장점이 있다고 생각합니다!

private final Persistence store = new ResultStore();

public void execute(Input input, Print printer) throws IOException {
this.input = input;
this.printer = printer;
public CalculatorApp() {
initStrategies();
}

private void initStrategies() {
strategies.put("1", new CalculationStrategy(
new InfixValidator(), new InfixToPostfixConverter(), new PostfixCalculator()));
Comment on lines +27 to +28
Copy link

Choose a reason for hiding this comment

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

구현체에 직접 의존하게 된다면,� 객체지향의 장점을 살리기 힘들 것 같은데 어떻게 생각하시나요?

}

public void execute(Input input, Print printer) {

while (true) {
String selection = input.getInput();
String selection = inputMenuSelection(input);
CalculatorStrategy selectedStrategy = getStrategyOrThrow(selection);
performStrategy(input, printer, selectedStrategy);
}
}

private void performStrategy(Input input, Print printer, CalculatorStrategy selectedStrategy) {
selectedStrategy.execute(input, printer, store);
}

private CalculatorStrategy getStrategyOrThrow(String selection) {
CalculatorStrategy selectedStrategy = strategies.get(selection);
if (selectedStrategy == null) {
throw new IllegalArgumentException(CalculatorMessage.INVALID_INPUT.message);
}
return selectedStrategy;
}
Comment on lines +44 to +50
Copy link

Choose a reason for hiding this comment

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

OrThrow라는 이름은 특정 상황이 만족하면 예외를 던질 수도 있다고 느껴져요.
�getStrategy()로 충분할 것 같습니다.


private String inputMenuSelection(Input input) {
return input.getInput();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.wonu606.calculator.calculator;

import java.util.List;

public interface Calculator {

double calculate(List<String> expression);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.wonu606.calculator.calculator;

import com.wonu606.calculator.model.Operator;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.List;
import java.util.Objects;

public class PostfixCalculator implements Calculator {

@Override
public double calculate(List<String> postfixExpression) {
Deque<Double> operandStack = new ArrayDeque<>();

for (String token : postfixExpression) {
handleToken(operandStack, token);
}

return operandStack.pop();
}

private void handleToken(Deque<Double> operandStack, String token) {
if (isOperator(token)) {
Operator operator = Objects.requireNonNull(Operator.get(token));
Copy link

Choose a reason for hiding this comment

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

Objects.requireNonNull()로 검사하기 보다는, Operator에 책임을 두는편이 더 좋아보여요

performOperation(operandStack, operator);
return;
}
operandStack.push(Double.valueOf(token));
}

private void performOperation(Deque<Double> operandStack, Operator operator) {
double secondOperand = operandStack.pop();
double firstOperand = operandStack.pop();
Comment on lines +32 to +33
Copy link

Choose a reason for hiding this comment

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

stack이 비어있다면 어떻게 되나요?


double result = operator.apply(firstOperand, secondOperand);
operandStack.push(result);
}

private boolean isOperator(String token) {
return Operator.get(token) != null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.wonu606.calculator.converter;

public interface Converter<T, R> {

R convert(T input);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package com.wonu606.calculator.converter;

import com.wonu606.calculator.model.Operator;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Deque;
import java.util.List;
import java.util.Objects;

public class InfixToPostfixConverter implements Converter<String, List<String>> {

@Override
public List<String> convert(String infixExpression) {
Deque<String> operatorStack = new ArrayDeque<>();
List<String> postfixExpression = new ArrayList<>();

String[] tokens = infixExpression.split("\\s");
Copy link

Choose a reason for hiding this comment

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

만약 입력 요구사항이 달라지면 split은 어떻게 되나요?


for (String token : tokens) {
processTokenIntoPostfix(operatorStack, postfixExpression, token);
}

while (!operatorStack.isEmpty()) {
postfixExpression.add(operatorStack.pop());
}
return postfixExpression;
}

private void processTokenIntoPostfix(
Deque<String> operatorStack, List<String> postfixExpression, String token) {
if (isOperator(token)) {
popWhileHigherPrecedence(operatorStack, postfixExpression, token);
operatorStack.push(token);
return;
}
postfixExpression.add(token);
}

private void popWhileHigherPrecedence(
Deque<String> operatorStack, List<String> postfixExpression, String tokenOperator) {
while (!operatorStack.isEmpty()
&& isHigherOrEqualPrecedence(operatorStack.peek(), tokenOperator)) {
Comment on lines +41 to +42
Copy link

Choose a reason for hiding this comment

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

processTokenIntoPostfix부터 반복문이 굉장히 많은데, 성능적인 이슈는 없을까요?

postfixExpression.add(operatorStack.pop());
Copy link

Choose a reason for hiding this comment

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

Suggested change
postfixExpression.add(operatorStack.pop());
String operator = operatorStack.pop();
postfixExpression.add(operator);

되도록 가독성을 위해 분리해주세요

}
}

private boolean isHigherOrEqualPrecedence(String peekOperator, String tokenOperator) {
int peekPrecedence = Objects.requireNonNull(Operator.get(peekOperator)).precedence;
int tokenPrecedence = Objects.requireNonNull(Operator.get(tokenOperator)).precedence;
return peekPrecedence <= tokenPrecedence;
}

private boolean isOperator(String token) {
return Operator.get(token) != null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.wonu606.calculator.model;

import java.util.Objects;

public class CalculationResult {

private final String expression;
private final double result;

public CalculationResult(String expression, double result) {
this.expression = expression;
this.result = result;
}

public String getExpression() {
return expression;
}

public double getResult() {
return result;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
CalculationResult that = (CalculationResult) o;
return Double.compare(that.getResult(), getResult()) == 0 && Objects.equals(
getExpression(), that.getExpression());
}

@Override
public int hashCode() {
return Objects.hash(getExpression(), getResult());
}
Comment on lines +23 to +39
Copy link

Choose a reason for hiding this comment

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

equals와 hashCode를 재구현하신 이유가 뭘까요?

Copy link
Author

@wonu606 wonu606 Jun 12, 2023

Choose a reason for hiding this comment

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

재구현 이유

Object에서 상속받은 equlashashCode로는
CalculationResult를 비교할 수 없다고 생각하여 재구현하였습니다.

사용

저장소에 CalculationResult이 제대로 저장되었는지 확인할 때 사용하였습니다.

Choose a reason for hiding this comment

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

기본 equals와 hashCode 는 어떻게 구현되어있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

Object.equals

  • '=='와 같은 연산을 비교하여 기본 equals는 참조값 비교입니다.

Object.hashCode

해시코드가 native로 되어있어 정확한 코드를 보진 못했습니다.
하지만 공식 문서에서 두 객체의 equals에 따라 동일한 경우, 같은 hasocde가 나와야 한다고 되어있습니다.
내부적으로 해시함수에 따른 해시를 생성하고, 이후 equals를 통해 한번 더 비교하는 것이 아닌가 추측이 됩니다.

저는 자동 완성으로 equalshash를 만들어 기대 성능인 O(1)이 안나올 수도 있다고 생각하게 되었습니다.

Copy link

Choose a reason for hiding this comment

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

저장소에 CalculationResult이 제대로 저장되었는지 확인할 때 사용했다는 말은, 테스트 코드를 위해 비즈니스 로직을 변경한 것으로 보이는데 이게 올바른 방법일까요?

Copy link
Author

Choose a reason for hiding this comment

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

일반적으로 테스트 코드를 위해 비즈니스 로직을 변경한 것이 문제가 된다고 생각합니다.
하지만 CalculationResult는 값만을 가지고 있고, 값만을 가지고 있는 객체는 Value Object라고 하여
동등성이 보장되어야 한다고 생각하였습니다.

궁금한 점이 있습니다!! 인창 멘토님
VO 객체라도 equalshashcode를 사용 없이 구현하는 예외로 두면 안되는 건가요??
제가 그렇게 생각한 이유는 VO 객체에 두 메소드를 구현하지 않으면
추후 Set<CalculationResult>를 사용하고
CalculationResultequalshashcode이 구현되지 못한 것을 보고 사용할 경우
실수를 유발할 수 있다고 생각하기 때문입니다.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package com.wonu606.calculator.model;

import com.wonu606.calculator.util.CalculatorMessage;

public enum Operator {
ADD("+", 2) {
@Override
public double apply(double a, double b) {
return a + b;
}
},
SUBTRACT("-", 2) {
@Override
public double apply(double a, double b) {
Copy link

Choose a reason for hiding this comment

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

a와 b가 같은 타입이기 때문에 순서에 실수가 있다면, 전혀 다른 결과값을 받을 수도 있겠네요

return a - b;
}
},
MULTIPLY("*", 1) {
@Override
public double apply(double a, double b) {
return a * b;
}
},
DIVIDE("/", 1) {
@Override
public double apply(double a, double b) {
if (b == 0) {
throw new ArithmeticException(CalculatorMessage.NOT_DIVISIBLE_BY_ZERO.message);
}
return a / b;
}
};

public final String symbol;
public final int precedence;

Operator(String symbol, int precedence) {
this.symbol = symbol;
this.precedence = precedence;
}

public static Operator get(String symbol) {
for (Operator operator : values()) {
if (operator.symbol.equals(symbol)) {
return operator;
}
}
Comment on lines +43 to +47
Copy link

Choose a reason for hiding this comment

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

Stream을 활용해보는건 어떨까요?

return null;
Copy link

Choose a reason for hiding this comment

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

null을 반환하는 이유가 있을까요?

}

public abstract double apply(double a, double b);
Copy link

Choose a reason for hiding this comment

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

추상 메소드를 사용한 이유가 있을까요??

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.wonu606.calculator.storage;

import com.wonu606.calculator.model.CalculationResult;
import java.util.List;

public interface Persistence {

void saveResult(CalculationResult calculationResult);

CalculationResult findResult(int sequence);

List<CalculationResult> findAllResult();

void clear();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.wonu606.calculator.storage;

import com.wonu606.calculator.model.CalculationResult;
import com.wonu606.calculator.util.CalculatorMessage;
import java.util.ArrayList;
import java.util.List;

public class ResultStore implements Persistence {

private final List<CalculationResult> store = new ArrayList<>();

@Override
public void saveResult(CalculationResult calculationResult) {
store.add(calculationResult);
}

@Override
public CalculationResult findResult(int sequence) {
try {
return store.get(sequence - 1);
} catch (IndexOutOfBoundsException exception) {
throw new IllegalArgumentException(CalculatorMessage.INVALID_ORDER.message);
}
}
Comment on lines +18 to +24
Copy link

Choose a reason for hiding this comment

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

sequence를 입력받아 하나의 결과 값을 찾을 수 있군요?
이런 기능이 어떤 의미가 있을까요?


@Override
public List<CalculationResult> findAllResult() {
return new ArrayList<>(store);
}
Comment on lines +26 to +29
Copy link

Choose a reason for hiding this comment

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

새로운 인스턴스에 반환하는 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

그대로 반환해주게 된다면 외부에서 store 값을 수정할 수도 있겠다고 생각했습니다.
하지만 제가 원하는 동작이 아니여서 새로운 인스턴스로 반환하였습니다.
CalculationResult는 필드가 final이라 따로 객체를 생성하지는 않았습니다!

Copy link

Choose a reason for hiding this comment

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

List를 일급 컬렉션으로 만들어 반환하는 것도 고려해보시죠


@Override
public void clear() {
store.clear();
}
}
Loading