Skip to content
14 changes: 13 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1 +1,13 @@
# java-calculator-precourse
# java-calculator-precourse

# 기능 목록
*MVC에 따라 구현해 기능목록 외의 커밋 존재

1. 콘솔 입출력 관리 : Console
2. 구분자 인식 기능 : SeparatorRecognizeService
- 기본 구분자 인식
- 커스텀 구분자 저장 및 인식
3. 수 변환 및 합산 기능 : NumberProcessService
- 수 변환
- 트리거에 따라 저장된 변환 수 합산
4. 메인 예외처리 기능
4 changes: 4 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ java {
}
}

tasks.withType(JavaCompile) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

프로그래밍요구사항
프로그래밍요구사항에 build.gradle 파일은 변경할 수 없다고 나와있습니다:)
gradle.properties파일이나 IDE 설정을 변경하는 방법으로 변경하는 것이 좋을 것 같습니다

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

엄마야 까먹었었네요 알려주셔서 감사합니다~~

options.encoding = 'UTF-8'
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

한글이 깨져서 골치인데 혹시 해당 코드로 혹시 문제가 해결 되었나요..?

@daeGULLL daeGULLL Feb 9, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

넵!!! 저도 그 골치때문에 인터넷에 검색했더니 저게 답인 것 같더라구요 저거 이후에 테스트케이스 함수가 한국어라 생기던 오류가 잡혔습니다~
++엇 그런데 난슬님이 여기 수정하면 안된다 해서 gradle의 다른 파일을 만지는 방향으로 가야할 것 같습니다 저도 그렇게 수정할 예정입니다 @Regyung

repositories {
mavenCentral()
maven { url 'https://jitpack.io' }
Expand Down
14 changes: 13 additions & 1 deletion src/main/java/calculator/Application.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
package calculator;

import calculator.controller.CalculatorController;
import calculator.view.Console;

public class Application {
public static void main(String[] args) {
// TODO: 프로그램 구현
new Application().run();
}

public void run(){
Console console = new Console();
CalculatorController controller = new CalculatorController(console);

controller.input();
controller.process();
controller.output();
}
}
27 changes: 27 additions & 0 deletions src/main/java/calculator/Model/DetectResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package calculator.Model;

public class DetectResult {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

모델의 책임을 작게 잡으신 것 같아요...! 모델보다는 dto에 가까운 것 같아요!

@daeGULLL daeGULLL Feb 9, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

맞습니다! 솔직히 모델의 개념을 완벽히 이해하지 못해서 긴가민가하다 한 블로그 게시물에서 DTO형식대로 써뒀기에 일단 그대로 해뒀습니다...혹시 모델에 대해 추천해주실만한 글 있을까용? @moongua404

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

딱 맞는 글은 찾지 못했고... MVC 패턴에 대한 개괄적인 설명을 담은 글은 찾았습니다
MVC은 데이터를 담당하는 Model과 사용자와의 인터페이스 역할을 하는 View, 그 둘을 중계하는 Controller로 구성되어있습니다. 그 외에도 컨트롤러에 비지니스 로직이 섞이는 것을 막기 위해 Service, 기타 상수 관리, 검증 등의 잡무(?)를 맡는 Utility 등을 두기도 하는 것 같습니다. MVC의 핵심은 데이터의 로직을 Model이 처리하고 Controller는 최소한의 연결 역할만을 갖는다는 것 같습니다!
저 역시 디자인 패턴에 대해서는 잘 안다고 자부할 수 없기에 참고만 해 주세요

private int isDetected;
private int resultNum;

public DetectResult(){
this.isDetected = 0;
this.resultNum = 0;
}

public int getResultNum(){
return resultNum;
}

public int getDetected(){
return isDetected;
}

public void setDetected(int num){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

숫자로 상태를 정하니까 코드를 쓰지 않은 사람은 상태를 이해하는게 힘들 것 같아요, 만약 상태를 이런식으로 설정해야 한다면 Enum 타입을 활용하면 좀 더 가독성이 좋아질 것 같아요

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

안그래도 고민하며 int쓰면 간결한걸 굳이 늘려버리는거 아닐까 생각해 넘어갔었는데 가독성이 많이 떨어지나보군요..ㅋㅋㅠ 수정하겠습니다~~

this.isDetected = num;
}

public void setResultNum(int resultNum) {
this.resultNum = resultNum;
}
}
63 changes: 63 additions & 0 deletions src/main/java/calculator/controller/CalculatorController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package calculator.controller;

import calculator.Model.DetectResult;
import calculator.service.NumberProcessService;
import calculator.service.SeparatorRecognizeService;
import calculator.view.Console;

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

코드를 보니 사용되지 않는 import가 남아있는 것 같아 제거하면 좋을 것 같아요!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

헉 수정할때 잘 삭제를 안하다보니 남았나보네요 삭제하겠습니당~~!!

import java.util.List;
import java.util.Objects;

import static calculator.service.NumberProcessService.addAll;

public class CalculatorController {

private Console console;

private SeparatorRecognizeService separatorRecognizeService;

private static String rawArg;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

변수 이름을 쓸 때 축약하지 않고 좀 더 직관적인 이름이면 좋을 것 같아요! 개인적인 생각에는 inputLine 정도만 되어도 좀 더 직관적으로 이해할 수 있을 것 같아요

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

변수명을 짓는 건 항상 난제이지만 처음부터 너무 복잡하게 생각하지 않길 바라요!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

이건 제 능지이슈로 더 최선의 것을 떠올려내지 못했었습니다...지어주신 이름 잘 쓰도록 하겠습니다 감사합니다~~~

private List<Integer> numberList = new ArrayList<>();
private int calculationResult;
private DetectResult[] detectResult = new DetectResult[2];
private char temp;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Controller의 필드가 많고, 그 중 연산 중간에 필요한 필드가 꽤 있는 것 같아요.
함수를 분리하는 등 불필요한 필드를 줄이면 가독성이 높아질 것 같아요

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

오호 참고하겠습니다


public CalculatorController(Console console){
this.console = console;
separatorRecognizeService = new SeparatorRecognizeService();
}

public void input(){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이 부분은 Controller의 역할 보다는 View의 역할에 가까운 것 같아요

@daeGULLL daeGULLL Feb 9, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

저도 그렇게 생각했습니다..그런데 view는 기능을 가진 채 존재만 하고 뭔가 실제로 조작할때는 컨트롤러를 사용한다고 글을 읽어서 고민하다 컨트롤러 내부에 해당 함수를 만들었었습니다..! view 이놈도 뭔가 조작? 기능을 가져도 되는건가요? 인터넷에 올라온 글들만 봐선 MVC 패턴을 잘 이해를 못하겠드라구요ㅠㅠ @moongua404

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

뷰에 비지니스로직이 포함되면 안되는 것은 맞지만 자료형 변환, 때로는 최소한의 타입 검사 등을 포함시키는 경우도 있는 것으로 알고있습니다.
제가 이 코드를 해석하기에는 입출력 장치만을 가져와 console.print("덧셈할 문자열을 입력해 주세요.");, rawArg = console.read(); 등 입출력의 역할을 Controller가 가져간 것 같다고 이해했습니다

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

뷰에 비지니스로직이 포함되면 안되는 것은 맞지만 자료형 변환, 때로는 최소한의 타입 검사 등을 포함시키는 경우도 있는 것으로 알고있습니다.
제가 이 코드를 해석하기에는 입출력 장치만을 가져와 console.print("덧셈할 문자열을 입력해 주세요.");, rawArg = console.read(); 등 입출력의 역할을 Controller가 가져간 것 같다고 이해했습니다

console.print("덧셈할 문자열을 입력해 주세요.");
rawArg = console.read();
if(Objects.equals(rawArg, ""))calculationResult=0;
}

public void process(){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. numberList.add(num) 이 반복적으로 호출되는 점
  2. getDetected 값이 1일때 숫자를 리스트에 추가하는 로직이 반복되는 점
    같은 중복코드가 좀 있는 것 같아요!

-> detectResult를 검사하고 숫자를 리스트에 추가하는 공통 메서드를 추출하고 getDetected 값은 한번만 검사하도록 수정해보는 방향은 어떨까요??

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

안그래도 그게 제 목표입니당 제가 짜놓은 로직에 따라서 그게 될지 저도 잘 모르겠지만 꿀팁 잘 받아서 해당 방향으로 한번 시도해보도록 하겠습니다~~

int leng = rawArg.length();
for(int i=0; i<leng; i++) {
temp = rawArg.charAt(i);
detectResult[0] = separatorRecognizeService.specialDetect(temp, i, leng);

if(!(detectResult[0].getDetected()==2)) {
detectResult[1] = separatorRecognizeService.normalDetect(temp, i, leng);
if(detectResult[1].getDetected()==1) {
int num = detectResult[1].getResultNum();
numberList.add(num);
}
}
if(detectResult[0].getDetected()==1) {
int num = detectResult[0].getResultNum();
numberList.add(num);
}
}
calculationResult = NumberProcessService.addAll(numberList);
}

@moongua404 moongua404 Feb 8, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

코드보다 순서논리회로(?)를 보는 것 같아요...

  1. 의존성이 개념적으로 분리되지 않은 것 같습니다. 가령 여기서 동작 원리를 이해하려면 detectResult의 getDetected를 이해해야하는데 이걸 이해하려면 normalDetect 등의 메서드를 봐야하고,,,
  2. 역할도 분리가 명확히 되지 않은 것 같습니다. process에 복잡한 내부 로직이 연산자 추출, 연산 등 다양한 역할이 섞여있는 것 같습니다
  3. API로 대체 가능한 기능들이 꽤나 많습니다.
  4. 휴먼 에러가 발생하기 쉽울 것 같습니다. 인덱스를 통해 접근하는 것 부터 각 부분이 어떠한 기능을 하는지 파악하기 어려워 다른 사람이 코드를 다룰 때 실수가 많이 발생할 수 있을 것 같습니다.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

확실히 약간 파일들끼리 꼬리의 꼬리를 무는 형태처럼 보여서 코드를 읽는 것 뿐만 아니라 작성하는데도 자세히, 조심히 봐야 할 것 같아요...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

이때부터 시간이 촉박해 생각나는대로 그대로 쓰다보니 슨서논리회로가 되버린게 맞을겁니다 허허 API를 좀더 알아봐 잘 대체하고 잘 분리시켜보도록 하겠습니당


public void output(){
String content = "결과 : " + String.valueOf(calculationResult);
console.print(content);
}
}
18 changes: 18 additions & 0 deletions src/main/java/calculator/service/NumberProcessService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package calculator.service;

import java.util.List;

public class NumberProcessService {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

서비스를 static call하는 이유가 있을까요?

@daeGULLL daeGULLL Feb 9, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Utility class를 만들때는 굳이 인스턴스 생성이 필요없어 스태틱으로 하는 것이 좋다고 전에 글에서 읽어서 써봤습니당~~

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Service는 일반적으로 MVC 패턴에서 비지니스 로직을 담당하는 계층을 의미한다고 알고있습니다! 내용을 보니까 Utility처럼 쓰여서 코드의 내용으로는 static이 더 적절한 것 같네요...ㅎㅎ

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

아하 그럼 유틸로 분리하겠습니다~~

public static int stringToInt(String arg){
if(!arg.matches("\\d+")) throw new IllegalArgumentException("Invalid number format");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 메소드에서 입력값이 null인경우의 예외처리를 더 명확하게 하는것은 어떨까요??
if (arg == null || !arg.matches("\\d+") 이런식으로요!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

오 반영하겠습니다~~

return Integer.parseInt(arg);
}

public static String intToString(int num){
return String.valueOf(num);
}

public static int addAll(List<Integer> targetList){
return targetList.stream().mapToInt(Integer::intValue).sum();
}
}
119 changes: 119 additions & 0 deletions src/main/java/calculator/service/SeparatorRecognizeService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package calculator.service;

import calculator.Model.DetectResult;

public class SeparatorRecognizeService {

private String normalClause="";
private String specialClause="";

private String specialSeparator="";

private boolean normalDetected=false;
private int specialDetected=0;
private int specialState = 0;
private int specialSeparatorDetected=0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이쪽도 필드의 역할이 연산에 필요한 상태 관리인 것 같은데 서비스의 필드로 둘 필요는 없을 것 같아요

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

그 그러면 어디 필드로 두시는걸 추천하시나요? 상태관리 클래스로 분리하는게 나을까요? @moongua404

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

normalDetected 등의 필드가 이 서비스가 갖는 속성이라고 보기는 어려울 것 같아서 이러한 상태관리는 함수 내에서 처리하는게 좋을 것 같습니다.


public DetectResult normalDetect(char word, int index, int len){
DetectResult detectResult = new DetectResult();
if(normalDetected) {
if(word==':'||word==',') {
int result = NumberProcessService.stringToInt(normalClause);
normalDetected = false;
normalClause = "";

detectResult.setDetected(1);
detectResult.setResultNum(result);

}else{
normalClause += word;
detectResult.setDetected(0);
}

}else{
if(word==':'||word==',') {
normalDetected = true;
}else if(Character.isDigit(word)){
normalClause+=word;
normalDetected=true;
}else{
throw new IllegalArgumentException("Invalid string format");
}
detectResult.setDetected(0);
}

if(len-1==index){
int result = NumberProcessService.stringToInt(normalClause);
normalDetected = false;
normalClause = "";

detectResult.setDetected(1);
detectResult.setResultNum(result);
}

return detectResult;
}

public DetectResult specialDetect(char word, int index, int len){
DetectResult detectResult = new DetectResult();
switch (specialState) {
case 0:
detectResult.setDetected(0);
if (specialSeparator!=""&&word == specialSeparator.charAt(specialSeparatorDetected)) {
detectResult.setDetected(2);
specialSeparatorDetected++;

if (specialSeparator.length() > 1 && specialSeparator.length() == specialSeparatorDetected) {
specialSeparatorDetected = 0;
specialDetected = 1;
} else if (specialDetected==1) {
int result = NumberProcessService.stringToInt(specialClause);
specialDetected = 0;
specialClause = "";

detectResult.setDetected(1);
detectResult.setResultNum(result);
}

} else if (specialDetected==1) {
detectResult.setDetected(2);
specialClause += word;
if(len-1==index){
int result = NumberProcessService.stringToInt(specialClause);
specialDetected = 0;
specialClause = "";

detectResult.setDetected(1);
detectResult.setResultNum(result);
}
} else if (word == '/') {
detectResult.setDetected(2);
specialState++;
}
else specialSeparatorDetected = 0;

return detectResult;
case 1:
if (word == '/') specialState++;
else throw new IllegalArgumentException("Invalid string format");

detectResult.setDetected(2);
return detectResult;
case 2:
if (word == '\\') specialState++;
else specialSeparator += word;

detectResult.setDetected(2);
return detectResult;
case 3:
if (word == 'n') specialState = 0;
else throw new IllegalArgumentException("Invalid string format");

detectResult.setDetected(2);
return detectResult;
}
detectResult.setDetected(0);
return detectResult;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

위에 함수도 그렇고 이 함수도 너무 길이가 긴 것 같아요! 코드의 흐름이 한 눈에 보이지 않을 뿐더러 여러 책임을 한 함수가 모두 지고있는 것 같아서 분리하는 쪽이 더 좋을 것 같습니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

넵~~

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

해당 코드들을 정규표현식을 활용해서 구현한다면 길이를 확 줄일 수 있어 보이고 더 효율적일 것 같습니다!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

맞아요 정규표현식 얘기가 이번 과제 통틀어서 많더라구요 한번 공부해서 줄여보겠습니다 감사합니다!!

11 changes: 11 additions & 0 deletions src/main/java/calculator/view/Console.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package calculator.view;

public class Console {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

뷰도 책임이 적은 것 같아요, 좀 더 역할을 부여해도 좋을 것 같아요

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

오호 혹시 역할의 예시를 조금 알려주실수 있을까요?? @moongua404

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

print에서 인수로 받은 메시지를 출력하는 것 보다는 printGuide, printResult로 안내 및 결과 출력을 담당하도록 만드는게 좋을 것 같습니다. 이렇게 되면 입출력의 관심사가 View로 온전히 분리될 수 있기 때문입니다.
사람에 따라 다른 것 같긴 한데 read()라는 입력 함수에도 안내 설명 등의 출력을 포함시키기도 한다고 알고 있습니다.

public String read(){
return camp.nextstep.edu.missionutils.Console.readLine();
}

public void print(String arg){
System.out.println(arg);
}
}