Skip to content

[보민] 문자열 덧셈 계산기 #12

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 3 commits into
base: jbm
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
5 changes: 4 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@ group 'com.ccstudy'
version '1.0-SNAPSHOT'

sourceCompatibility = 1.8
compileJava.options.encoding = 'UTF-8'

repositories {
mavenCentral()
}

dependencies {
compile('org.junit.jupiter:junit-jupiter:5.6.0')
testCompile('org.junit.jupiter:junit-jupiter:5.6.0')
testCompile group: 'org.assertj', name: 'assertj-core', version: '3.15.0'
testCompile "org.mockito:mockito-core:1.+"
}

test{
useJUnitPlatform()
}
}
11 changes: 11 additions & 0 deletions src/main/java/calculator/Calculator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package calculator;

public class Calculator {
public static int addValues(String[] tokens, Parser parser) {
int result = 0;
for (int i = 0; i < tokens.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

여기는 foreach나 stream을 사용해도 좋다고 생각합니다.
과하다 생각할 순 있지만 익숙해지는데에 연습하는것이 좋아보여요 !

Copy link
Author

Choose a reason for hiding this comment

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

넵! 익숙해져볼게욥...!!

result += parser.stringToInteger(tokens[i]);
}
return result;
}
}
11 changes: 11 additions & 0 deletions src/main/java/calculator/GetInput.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package calculator;

import java.util.Scanner;

public class GetInput {
public static String getInput() {
System.out.println("input : ");
Scanner scanner = new Scanner(System.in);
return scanner.nextLine();
}
}
18 changes: 18 additions & 0 deletions src/main/java/calculator/InputChecking.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package calculator;

public class InputChecking {

public static boolean checkIsTheInputBlank(String value) {

Choose a reason for hiding this comment

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

boolean형 함수에서 false가 나올수 없는 함수는 이상한 함수입니다
이런 함수는 void validate하는 함수로
if(value == null || value.isEmpty(){
Exception();
}
요게 더 좋아보이네요
그리고 nullpointerException보다는 IlleganAgumentException이 더 명시적으로 좋은 익셉션인거같습니다

if (value == null || value.isEmpty()) {
System.out.println("0");
throw new NullPointerException();
}
return true;
}

public static void checkSinglularityAndPrintResult(String inputValue, Parser singleParser) {

Choose a reason for hiding this comment

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

인풋값 체크하는 함수에 프린트용 함수가 있는 건 이상합니다

if (inputValue.length() == 1) {
System.out.println("result : " + singleParser.parseSingleValue(inputValue));
}
}
}
33 changes: 33 additions & 0 deletions src/main/java/calculator/Parser.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package calculator;

public class Parser {
public int stringToInteger(String stringToInt) {
int toInt = 0;
try {
toInt = Integer.parseInt(stringToInt);
checkTheValueIsPositive(toInt);
return toInt;

Choose a reason for hiding this comment

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

여기선 리턴이 없어도 되겠네요

} catch (NumberFormatException numberFormatException) {
System.out.println("Error Message : " + numberFormatException.getMessage());
}
return toInt;
}

private void checkTheValueIsPositive(int number) {
if (number < 0) {
System.out.println("양수를 입력하세요");
throw new IllegalArgumentException();
}
}

public int parseSingleValue(String inputValue) {
int result = 0;
try {
result = Integer.parseInt(inputValue);
} catch (NumberFormatException parsingFail) {
System.out.println("숫자를 입력하세요");
System.exit(0);
}
return result;
}
}
30 changes: 30 additions & 0 deletions src/main/java/calculator/Splitter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package calculator;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class Splitter {
public static final String DEFAULT_DELIMITER = "[,:]";
public static final String CUSTOM_DELIMITER_REGEX = "//(.)\n(.*)";
public static final int DELIMETER_GROUP = 1;
public static final int TOKEN_GROUP = 2;

private Pattern pattern = Pattern.compile(CUSTOM_DELIMITER_REGEX);

Choose a reason for hiding this comment

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

해당 객체는 상수로 존재하는게 좋아요
싱글톤 객체로 존재할때 효율이 좋아질거에요. Splitter객체를 생성할때마다 pattern이 생성되니 gc효율은 여전히 안좋아요
static final로 어플리케이션에 단하나만 존재하는 인스턴스로 만들어서 사용하는게 좋아요


public String[] getTokens(String text) {
if (text.contains(DEFAULT_DELIMITER)) {
return splitByDefaultDelimiter(text);
}
return splitByCustomDelimeter(text);
}

private String[] splitByDefaultDelimiter(String text) {
return text.split(DEFAULT_DELIMITER);
}

private String[] splitByCustomDelimeter(String text) {
Matcher matcher = pattern.matcher(text);
String customDelimeter = matcher.group(DELIMETER_GROUP);
return matcher.group(TOKEN_GROUP).split(customDelimeter);
}
}
17 changes: 17 additions & 0 deletions src/main/java/calculator/StringAddingCalculatorApplication.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package calculator;

public class StringAddingCalculatorApplication {
public static void main(String[] args) {

Choose a reason for hiding this comment

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

객체간의 관계가 어색합니다!
메인 메서드에서 객체간의 관계를 설정하고 있어요.

인풋 받기 -> 체크 -> 파서생성 -> 체킹클래스에서 파서 찍기 ->....

객체간의 관계가 없이 메인안에서 함수의 인자로 넘기거나 상태값도 메인 메서드에서만 유지하는 지역변수로 활용하고 있어서
구현되어져 있는 클래스만 보고는 요구사항을 파악하기 굉장히 힘들어보여요!

지금 클래스의 관계는 전부 static 메서드로 만들어도 표현할 수 있습니다.
이런 관계보단 Calculator라는 클래스안에 결과라는 멤버변수를 두고 Splitter와 Parser를 Calculator내부에서 사용하도록 하는게 어떨까요

String inputValue = GetInput.getInput();

InputChecking.checkIsTheInputBlank(inputValue);

Parser parser = new Parser();
InputChecking.checkSinglularityAndPrintResult(inputValue, parser);

Splitter splitter = new Splitter();
String[] tokens = splitter.getTokens(inputValue);

System.out.println("result : " + Calculator.addValues(tokens, parser));
}
}
28 changes: 28 additions & 0 deletions src/test/java/calculator/InputCheckingTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package calculator;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

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

class InputCheckingTest {


@DisplayName("인자의 값이 빈칸 또는 null 인지 점검한다.")
@Test
void shouldCheckIsTheInputBlankTest() {
Copy link

Choose a reason for hiding this comment

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

Null 또는 빈 문자열 테스트는 junit5의 @ParameterizedTest와 @NullAndEmptySource 로 간단하게 할 수 있어요!

    @ParameterizedTest
    @NullAndEmptySource
    void name(String isBlankOrNull) {
    }

이런식으로요! 또 checkIsTheInputBlank 함수는 NullPointerException throw 하니까 assertThat 보다는
assertThatThrownBy 를 이용하면 좋을 것 같아요!

String isBlank = "";
Copy link

Choose a reason for hiding this comment

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

String isNull = null;
Copy link
Member

Choose a reason for hiding this comment

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

isㅇㅇㅇ 이런 류의 명명은 Boolean 타입의 변수나 이 타입을 리턴하는 메소드에서 쓰이는 것이라,
isNull은 이름이 이상하다고 생각합니다.

assertThat(InputChecking.checkIsTheInputBlank(isBlank)).isTrue();
assertThat(InputChecking.checkIsTheInputBlank(isNull)).isTrue();
}

@DisplayName("인자의 값이 한 글자일 때, 글자수를 올바르게 판단하는 지 점검")
@ParameterizedTest
@ValueSource(strings = {"1", "2", "3", "23", "12"})
void shouldCheckSinglularityTest(String values) {
assertThat(InputChecking.checkSinglularityAndPrintResult(values)).isTrue();
}
}
47 changes: 47 additions & 0 deletions src/test/java/calculator/ParserTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package calculator;

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

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

class ParserTest {
Parser parser = new Parser();

@DisplayName("String을 Integer로 올바르게 파싱하는 지 점검")
@Test
void stringToInteger() {
String stringToInt = "3";
assertThat(parser.stringToInteger(stringToInt)).isEqualTo(3);
}

@DisplayName("인자가 문자일 때, 터지는 exception 점검.")
@Test
void shouldThrowException() {
String invalidValue = "j";
assertThat(parser.stringToInteger(invalidValue)).isEqualTo(new NumberFormatException());
}

@DisplayName("양수인지 점검")
@Test
void shouldCheckPositiveValueTest() {
String negativeInt = "-1";
assertThatThrownBy(() -> new Parser().stringToInteger(negativeInt))
.isInstanceOf(IllegalArgumentException.class);
}

@DisplayName("문자 하나를 입력했을 때, 제대로 파싱되는 지")
@Test
void singleValueTest() {
String singleValue = "1";
assertThat(parser.parseSingleValue(singleValue)).isEqualTo(1);
}

@DisplayName("숫자가 아닌 값을 입력했을 때, 오류")
@Test
void invalidSingleValueTest() {
String invalidSingleValue = "j";
assertThat(parser.parseSingleValue(invalidSingleValue)).isEqualTo(new NumberFormatException());
}
}
26 changes: 26 additions & 0 deletions src/test/java/calculator/SplitterTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package calculator;

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

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

class SplitterTest {
Splitter splitter = new Splitter();

@DisplayName("Custom delimeter test")
@Test
void customDelimeterTest() {
String inputText = "//;\n1;2;3";
String[] inspectTokens = {"1", "2", "3"};
assertThat(splitter.getTokens(inputText)).isEqualTo(inspectTokens);
}

@DisplayName("Default delimeter test")
@Test
void defaultDelimeterTest() {
String inputText = "1,2:3";
String[] inspectTokens = {"1", "2", "3"};
assertThat(splitter.getTokens(inputText)).isEqualTo(inspectTokens);
}
}