Skip to content
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

Step1 - 문자열 계산기 #1122

Open
wants to merge 6 commits into
base: lee-won-suk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions src/main/kotlin/calculator/Main.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package calculator

import java.util.*

fun main() {
println("계산식을 입력해주세요.")
val sc = Scanner(System.`in`)
val result = StringCalculator.calculate(sc.nextLine())
println(result);
}
46 changes: 46 additions & 0 deletions src/main/kotlin/calculator/StringCalculator.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package calculator

data class StringCalculator(val totalNumber: Int) {
Copy link

Choose a reason for hiding this comment

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

StringCalcualtor를 data class로 두셨군요?
data class를 사용하신 이유가 있으실까요? 🤔

Copy link

Choose a reason for hiding this comment

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

StringCalculatortotalNumber 라는 상태 값을 가지는 것으로 보이는데, 이 상태를 사용하는 위치는 없어보이네요. 🤔
상태 값이 필요하지 않다면 굳이 둘 필요가 없지 않을까요? 😃

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다. 생각해보니 테스트에서 상태값을 따로 쓰지도 않고 객체끼리 비교등도 하지 않는데 굳이 싶네요
data class의 장점은 불필요한 보일러플레이트 제거라고 생각하는데요, 현시점에서 사용되지는 않으니 제거 처리하였습니다


companion object {
fun calculate(input: String?): Int {
Copy link

Choose a reason for hiding this comment

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

kotlin은 nullable type과 non-nullable type을 구분하기 때문에, 애초에 입력으로 null을 재한하는 것이 좋다고 생각해요.

이 부분은 미션 요구사항에서 null인 경우 0을 반환한다고하는 조건이 있기 때문에 이를 위한 null 입력을 받는 케이스를 고려하신 것으로 이해했습니다. 😃

Copy link
Author

Choose a reason for hiding this comment

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

넵 맞습니다. 미션상에 null인경우 0반환 조건이 있어서 받는 케이스를 고려했습니다 :)

if (input.isNullOrBlank()) {
return 0
}

if (input.length == 1 && input.all { it.isDigit() }) {
return input.toInt()
}
Comment on lines +11 to +13
Copy link

Choose a reason for hiding this comment

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

굳이 여기에 이런 1개의 숫자만있는 케이스에 대해 분기처리하지 않아도, 동일한 결과를 얻을 수 있을 것 같습니다. 😃

Copy link
Author

Choose a reason for hiding this comment

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

세상에 그렇네요;
감사합니다 제거 처리했습니다.


val (delimiter, text) = parseDelimiterAndText(input)
validate(text, delimiter)
return splitString(text, delimiter)
}

private fun parseDelimiterAndText(input: String?): Pair<String, String?> {
Copy link

Choose a reason for hiding this comment

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

StringCalcualtor가 여러 역할을 수행하고있는 것 같은데요. 🤔
구분자수식을 구분하는 역할은 다른 객체에게 위임해보면 어떨까요? 😃

var delimiter = "[,:]"
var text = input
val result = text?.let { Regex("//(.)\n(.*)").find(it) }
Copy link

Choose a reason for hiding this comment

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

Regex는 꽤 비싼 객체인데요. 아래의 글을 참고해서 생성 방식을 개선해보면 좋을 것 같습니다. 😃

객체의 생성과 파괴 - 불필요한 객체 생성을 피하라

Copy link
Author

Choose a reason for hiding this comment

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

좋은 조언 감사합니다 (_ _) 캐싱전에 일단 컴패니언 객체밖에 없어서 object로 변경했습니다.
인스턴스화를 방지해보기 위해서입니다.
그 다음에 val로 정규표현식을 선언해서 캐싱해보았습니다.

result?.let {
delimiter = it.groupValues[1]
Copy link

Choose a reason for hiding this comment

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

기능 요구사항에 따르면

앞의 기본 구분자(쉼표, 콜론) 외에 커스텀 구분자를 지정할 수 있다. 커스텀 구분자는 문자열 앞부분의 “//”와 “\n” 사이에 위치하는 문자를 커스텀 구분자로 사용한다.

같이 말하고있어요. 즉, 커스텀 구분자가있다고해서 커스텀 구분자로만 구분이되는 것이 아닌 기본 구분자 + 커스텀 구분자로 구분이 되는 것이 요구사항인 것으로 보여요. 🤔
지금의 로직은 delimiter를 완전히 커스텀 구분자로 변경하는 것 같아서 의도한대로 동작하지 않을 것 같습니다.

테스트 코드에 “//;\n1,2:3;4” 와 같이 기본 구분자 + 커스텀 구분자가 섞인 요소도 추가하고 테스트를 통과하도록 로직을 수정해보시면 좋을 것 같습니다. 😉

Copy link
Author

Choose a reason for hiding this comment

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

좋은 말씀 감사합니다
커스텀 구분자가 추가되더라도 기본 구분자도 같이 검증하도록 수정했습니다

text = it.groupValues[2]
Comment on lines +25 to +26
Copy link

Choose a reason for hiding this comment

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

이 코드를 처음보는 살마의 입장에서는 1, 2와 같은 숫자를 한눈에 파악하기 어려울 수 있을 것 같아요.

의미가 불분명한 매직 넘버를 상수로 선언하라.

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다
둘다 인덱스를 나타내는 것이니 CUSTOM_DELIMITER_INDEX와 같이 표현해보았습니다.

}
return Pair(delimiter, text)
Copy link

Choose a reason for hiding this comment

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

Pair객체를 만들고 싶다면 to infix 함수를 활용할 수도 있�어요! 😃

public infix fun <A, B> A.to(that: B): Pair<A, B> = Pair(this, that)
Suggested change
return Pair(delimiter, text)
return delimiter to text

}

private fun validate(input: String?, delimiter: String) {
if (
Regex("[^\\d\n$delimiter//]").containsMatchIn(input.toString())) {
throw RuntimeException()
}
}

private fun splitString(
inputString: String?,
delimiter: String,
): Int {
return inputString?.split(Regex(delimiter))?.map(String::toInt)
?.sum() ?: 0
}
}
}
57 changes: 57 additions & 0 deletions src/test/kotlin/StringCalculatorTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import calculator.StringCalculator
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatRuntimeException
import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.CsvSource
import org.junit.jupiter.params.provider.NullAndEmptySource
import org.junit.jupiter.params.provider.ValueSource

class StringCalculatorTest {
@ParameterizedTest
@DisplayName(value = "쉼표, 콜론을 구분자로 분리한다.")
@CsvSource(value = ["'1,2',3", "'1:3',4"])
fun stringSplitTest(
inputString: String,
expacted: Int,
) {
assertThat(StringCalculator.calculate(inputString)).isEqualTo(expacted)
}

@ParameterizedTest
@DisplayName(value = "커스텀 구분자가 있으면 그 구분자로 분리한다.")
@CsvSource(value = ["'//;\n1;2',3", "'//_\n1_3',4"])
fun customSplitTest(
inputString: String,
expacted: Int,
) {
assertThat(StringCalculator.calculate(inputString)).isEqualTo(expacted)
}

@ParameterizedTest
@DisplayName(value = "음수 및 숫자 아닌값은 에러처리한다.")
@CsvSource(value = ["'-1:2',3", "'//_\n1@3',4"])
fun SplitfailTest(
inputString: String,
expacted: Int,
) {
assertThatRuntimeException().isThrownBy {
StringCalculator.calculate(inputString)
}
}

@DisplayName(value = "숫자 하나를 문자열로 입력할 경우 해당 숫자를 반환한다.")
@ParameterizedTest
@ValueSource(strings = ["1"])
fun oneNumber(text: String) {
assertThat(StringCalculator.calculate(text)).isSameAs(Integer.parseInt(text));
}


@DisplayName(value = "빈 문자열 또는 null 값을 입력할 경우 0을 반환해야 한다.")
@ParameterizedTest
@NullAndEmptySource
fun emptyOrNull(text: String?) {
assertThat(StringCalculator.calculate(text)).isZero();
}
}
13 changes: 13 additions & 0 deletions step1README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# 문자열 덧셈 계산기

기능 요구사항
1. 쉼표, 콜론을 구분자로 가지는 문자열을 전달하는 경우 구분자를 기준으로 분리한 각 숫자의 합을 반환한다.(""=>0, "1,2=>3, "1,2,3"=>6, "1,2:3"=>6)
2. - [x] 쉼표, 콜론를 기본 구분자로 판단하여 분리하여 리스트에 저장.or 바로 더하기
2. 앞의 기본 구분자외 커스텀 구분자 지정 가능. //와 \n 사이 위치문자를 커스텀 구분자로 사용한다.
3. - [x] //와 \n이 둘다 있을 경우 \\다음 문자를 구분자로 간주하고 분리
3. 문자열 계산기에 숫자이외 값 or 음수 전달시 RuntimeException throw
4. - [x] 구분자로 분리하고 순회하면서 숫자 이외값 있을시 throw. 이때 구분자랑 커스텀구분자는 제외 필요


구조
문자열 계산기 클래스