[4기 - 강병곤] 1~2주차 과제 : 계산기 구현 미션 제출합니다. by Curry4182 · Pull Request #189 · prgrms-be-devcourse/java-calculator

Curry4182 · 2023-06-15T10:48:01Z

📌 과제 설명

HashMap을 이용하여 계산 이력을 저장 하였습니다.
계산 이력 기능이 정상 작동하는지 테스트 케이스를 작성하였습니다.
역할과 책임을 생각하며 코드를 작성하였습니다.
CalcManager 객체 생성을 팩토리 메서드 패턴을 활용해서 생성하도록 만들었습니다.
식을 입력하면 계산 사칙연산 계산 결과가 나오도록 하였습니다.
내부적으로 후위연산으로 변경하고 계산결과가 출력되도록 하였습니다.

👩‍💻 요구 사항과 구현 내용

사용자에게 콘솔로 입력받은 계산 식을 맵으로 저장
-> 애플리케이션이 동작하는 동안 데이터베이스 외에 데이터를 저장할 수 있도록 개발 하였습니다.
입력 받은 데이터를 저장하고 불러오는 테스트 코드 작성

✅ 피드백 반영사항

필요하지 않는 의존성 주입 수정 (CalcConsoleView 클래스)
메서드 레퍼런스를 사용하여 가독성 있게 만들었습니다.
main 함수에 너무 많은 인스턴스가 생성되어 facory 클래스에서 생성 하도록 수정하였습니다.
Test 케이스에 given, when, then의 의미를 생각하고 다시 수정하였습니다.
식을 토큰 단위로 나누고 계산 하도록 구현
계산 결과 검증 테스트 구현

✅ PR 포인트 & 궁금한 점

main함수에서 팩토리 메서드를 활용하여 CalcManager를 생성하도록 코드를 수정하였습니다. 처음에는 repository, calculator, view를 하나의 팩토리에서 생성하도록 만들다가 서로 의존적이지 않는 클래스라고 생각해서 분리 하여 만들었습니다.
코드를 작성하면 팩토리 메서드 처럼 객체 생성을 분리하는것에 대해 확신이 점점 들지 않았습니다. 기존에 main에서 만드는 것보다 복잡해졌지만 효율적인가에 대해 고민했습니다.
코드의 확장성을 위해 지금 처럼 구현하는게 맞을지 아니면 기존 코드처럼 main에서 CalaManager와 관련된 repository, view, Calculator를 생성 하지만 내부적으로 생성할 수 있는 의존성들은 내부에서 생성하도록 구현하는게 좋을지 궁금합니다!!

…클래스에 하도록 위임

ICCHOI

병곤님 과제 하느라 수고하셨습니다!
1주차보다 많이 발전하신 것 같아요.
New팀에서도 이렇게 적극적으로 임하시면 금방 성장 하실거에요 :)

ICCHOI · 2023-06-16T12:39:53Z

src/main/java/calcproject/di/container/CalcManagerDependencyInjectionContainer.java

+import calcproject.view.CalcInput;
+import calcproject.view.CalcOutput;
+
+public class CalcManagerDependencyInjectionContainer {


DependencyInjectionContainer라는 네이밍은 너무 스프링 기술에 치중되어 있는 느낌이 드는데요,
실제로 스프링의 기술을 구현해서 사용하는 것이 아니라면, 다른 이름이 좋아보여요

ICCHOI · 2023-06-16T16:11:25Z

src/main/java/calcproject/Main.java

 		CalcExpressionTokenizer calcExpressionTokenizer = new CalcExpressionTokenizer();
 		Calculator calculator = new Calculator(calcExpressionTokenizer);

-		CalcManager calcManager = new CalcManager(calcResultRecordRepository, calcConsoleView, calcConsoleView,
-			calculator);
+		CalcManagerViewFactory calcManagerViewFactory = new ConsoleViewCalcManagerViewFactory();
+		CalcResultRecordRepositoryFactory calcResultRecordRepositoryFactory = new InMemoryCalcResultRecordRepositoryFacotry();
+
+		CalcManagerDependencyInjectionContainer calcManagerDependencyInjectionContainer =
+			new CalcManagerDependencyInjectionContainer(calcResultRecordRepositoryFactory, calcManagerViewFactory,
+				calculator);


모든 객체가 main을 통해서 생성되고 있어서, main과 강한 의존성을 가지겠네요.
구조를 짜고 계층형으로 분리하면 좀 더 좋을 것 같아요.

ICCHOI · 2023-06-16T16:31:29Z

src/main/java/calcproject/repository/MemoryCalcResultRecordRepository.java

@@ -12,9 +12,9 @@ public class MemoryCalcResultRecordRepository implements CalcResultRecordReposit
 	private Map<Integer, CalcResultRecordModel> calcMap;


입력과 조회(모든 레코드) 두 가지 기능만 사용하고 있는데 굳이 Map을 쓰신 이유가 있나요?

ICCHOI · 2023-06-16T16:32:50Z

src/main/java/calcproject/view/Command.java

 	public static Command valueOf(int choiceNum) {
 		return Arrays.stream(values())
-			.filter(value -> value.equals(choiceNum))
+			.filter(value -> value.getCmdIdx() == choiceNum)


비교하는 메서드를 따로 만드는건 어떨까요?

ICCHOI · 2023-06-16T16:35:11Z

src/main/java/calcproject/view/Command.java

-			.filter(value -> value.equals(choiceNum))
+			.filter(value -> value.getCmdIdx() == choiceNum)
 			.findAny()
 			.orElse(CALCULATE.EXIT);


CALCULATE.EXIT는 사용자 의도로 인해 종료한다는 느낌인데요
강제종료를 의도하신거라면 TERMINATE는 어떠신가요?

ICCHOI · 2023-06-16T16:56:15Z

src/main/java/calcproject/engine/Calculator.java

+				if (stackPeekOPerator == Operator.UnSupportedOp) {
+					return null;
+				}


List을 반환값으로 가지고 있는데 null을 반환하는 이유가 있을까요?

ICCHOI · 2023-06-16T17:04:18Z

src/main/java/calcproject/engine/Calculator.java

+		for (int i = 0; i < postFixNotationTokens.size(); i++) {
+			String token = postFixNotationTokens.get(i);


for each를 사용해도 되겠네요

ICCHOI · 2023-06-16T17:05:45Z

src/main/java/calcproject/engine/Calculator.java

+				double operand2 = stack.pop();
+				double operand1 = stack.pop();


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

ICCHOI · 2023-06-16T17:11:26Z

src/test/java/calcproject/engine/CalcExpressionTokenizerTest.java

+	private static Stream<Arguments> expressionProvider() {
+		return Stream.of(
+			Arguments.of("1 + 2 * 3 / 4", Arrays.asList("1", "+", "2", "*", "3", "/", "4")),
+			Arguments.of("10 * 4 / 1", Arrays.asList("10", "*", "4", "/", "1")),
+			Arguments.of("4 * 3 / 2 - 1", Arrays.asList("4", "*", "3", "/", "2", "-", "1"))
+		);
+	}


MethodSource를 사용해보셔군요!
그런데 테스트에 어떤 값을 넣었는지 확인하기는 좀 불편한 것 같아요

ICCHOI · 2023-06-16T17:12:56Z

src/test/java/calcproject/repository/MemoryCalcResultRecordRepositoryTest.java

+		for (int i = 0; i < expressionList.size(); i++) {
+			String expression = expressionList.get(i);
+			Double calcRsult = calcResultList.get(i);
+
+			CalcResultRecordModel calcResultRecord = new CalcResultRecordModel(expression, calcRsult);
+			this.calcResultRecordRepository.saveCalcResultRecord(calcResultRecord);
+			expectedCalcRecords.add(calcResultRecord);


이런 코드가 테스트 코드에 들어가게 되면, CalcResultRecordModel 코드가 변경되면 전혀 상관없는 MemoryCalcResultRecordRepository의 테스트 코드가 깨지게 되겠네요

refactor: CalcManager 객체 생성을 CalcManagerDependencyInjectionContainer …

ea304be

…클래스에 하도록 위임

Curry4182 added Team-Ray 🔥 매운맛 labels Jun 15, 2023

Curry4182 requested review from ICCHOI and devcourse-ray June 15, 2023 10:48

Curry4182 self-assigned this Jun 15, 2023

Curry4182 added 7 commits June 15, 2023 19:52

refactor: 구현 하지 않는 메서드 null 대신 빈 값을 반환하도록 수정

610619a

feat: 식을 토큰 으로 나누는 기능 및 테스트 코드 추가

bf3f864

style: 자바 코드 컨벤션에 맞게 코드 스타일 수정

218140d

feat: 계산 기능 추가 및 테스트 코드 추가

bbd270b

refactor: 테스트 코드 파라미터로 값 을 받을 수 있도록 수정

4b4de21

refactor: 외부에 값을 받지 않아도 되는 생성자 파라미터 삭제, fix: 테스트 실패 버그 수정

de08a26

style: 자바 코드 컨벤션에 맞게 코드 스타일 수정

8a8d6c2

ICCHOI reviewed Jun 16, 2023

View reviewed changes

Curry4182 added 2 commits June 17, 2023 03:02

refactor: 지원하지 않는 연산일 경우 null반환 대신 빈 리스트를 반환하도록 수정

9c15fa9

refactor: 팩토리로 CalcManager 객체 생성 기능 삭제

3c3a5e2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4기 - 강병곤] 1~2주차 과제 : 계산기 구현 미션 제출합니다.#189

[4기 - 강병곤] 1~2주차 과제 : 계산기 구현 미션 제출합니다.#189
Curry4182 wants to merge 10 commits intoprgrms-be-devcourse:Curry4182from
Curry4182:main

Curry4182 commented Jun 15, 2023 •

edited

Loading

Uh oh!

ICCHOI left a comment

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

ICCHOI Jun 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -12,9 +12,9 @@ public class MemoryCalcResultRecordRepository implements CalcResultRecordReposit
		private Map<Integer, CalcResultRecordModel> calcMap;

		for (int i = 0; i < postFixNotationTokens.size(); i++) {
		String token = postFixNotationTokens.get(i);

		double operand2 = stack.pop();
		double operand1 = stack.pop();

Conversation

Curry4182 commented Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 과제 설명

👩‍💻 요구 사항과 구현 내용

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

Uh oh!

ICCHOI left a comment

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!

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!

2 participants

Curry4182 commented Jun 15, 2023 •

edited

Loading