feat: step2 문자열 계산기 구현 #26

JiwonDev · 2023-08-14T17:24:53Z

객체지향 생활체조 규칙 지켜보려고 요리조리 해보다가, 그냥 포기하고 올립니다 ㅋㅋㅋㅋㅋ ㅠㅠ
회사 코드보다 더 어려운거 같아요.

아래 4개의 클래스로 역할을 나눴습니다.
CalculateUseCase : 실제 사용자가 계산기를 사용할 때 이용하는 객체입니다.

ExpressionTokenizer: 문자열로 된 입력을 토큰 리스트로 변환합니다.
ExpressionConvertor: 토큰 리스트를 계산가능한 표현식(Expression)으로 변환합니다.
ExpressionCalculator: 표현식(Expression)을 계산하여 값을 얻습니다.

대충 아래와 같이 책임을 나눴습니다. 😀

Tokenizer -> 잘못된 문자열 입력 예외처리, 입력값의 형태가 바뀌었을 때 처리
o 덧셈 기호를 + 가 아닌 T로 바꿔주세요
o 스페이스바를 무시하고 계산하도록 바꿔주세요
o 숫자를 이진수 형태로 입력받게 바꿔주세요

Convertor -> 연산자 우선순위, 유효한 계산식 형태등을 판단
o *, / 가 먼저 계산되도록 바꿔주세요
o (((-1))) + 1 이런 계산식도 처리하게 바꿔주세요

Calculator -> 산술연산 유효성 판단, 계산 노예
o 숫자를 0으로 나눴을 때 좀 더 자세한 예외 메시지를 남겨주세요
o 계산식을 쪼개서 병렬로 처리하도록 만들어주세요.

JiwonDev · 2023-08-14T17:33:36Z

src/main/kotlin/step2/application/CalculateUseCase.kt

+    fun calculate(stringExpression: String): Double {
+        require(stringExpression.isNotBlank()) { "빈 표현식은 계산할 수 없습니다." }
+
+        val tokens: List<RawToken> = expressionTokenizer.parse(expression = stringExpression)
+
+        val expression: Expression = expressionConvertor.convert(tokens = tokens)
+
+        require(expressionCalculator.isCalculable(expression)) { "계산할 수 없는 표현식 타입입니다." }
+        return expressionCalculator.calculate(expression = expression)
+    }
+}


RawToken은 단순히 문자열로 자른 토큰입니다.

이상한 문자가 들어와 자를 수 없다면 여기에서 예외를 반환합니다.
"100-1*5" -> [ RawToken("100"), RawToken("-"), RawToken("5") ]

이 RawToken들은 expressionConvertor을 거치며 계산가능한 토큰들로 변환되고, 표현식에 맞게 순서가 재정렬됩니다.

토큰화 되었는데, 이게 계산 가능한 ExpressionToken으로 변환할 수 없다면 여기에서 예외를 반환합니다.

Expression( type=ExpressionType.POSTFIX tokens=[ NumberToken(100), NumberToken(5), OperatorToken(PLUS) ] )

그리고 ExpressionCalculator에서 순서대로 계산을 합니다.

토큰은 유효하나, 산술 계산이 불가능하다면 여기에서 예외를 발생시킵니다.

JiwonDev · 2023-08-14T17:36:15Z

src/main/kotlin/step2/service/ExpressionConvertor.kt

+    override fun convert(tokens: List<RawToken>): Expression {
+        return tokens.map(::toExpressionToken)
+            .let { infixTokens: List<ExpressionToken> -> convertToPostfix(infixTokens) }
+            .let { postfixTokens: List<ExpressionToken> -> Expression(type = ExpressionType.POSTFIX, tokens = postfixTokens) }
+    }
+
+    private fun convertToPostfix(infixTokens: List<ExpressionToken>): List<ExpressionToken> {
+        val postfixExpression: MutableList<ExpressionToken> = mutableListOf()
+
+        infixTokens
+            .fold(initial = ArrayDeque()) { operatorStack: ArrayDeque<ExpressionToken>, token: ExpressionToken ->
+                when (token) {
+                    is NumberToken -> operatorStack.also { postfixExpression.add(token) }
+                    is OperatorToken -> {
+                        if (operatorStack.isNotEmpty()) postfixExpression.add(operatorStack.removeLast())
+                        operatorStack.apply { addLast(token) }
+                    }
+                }
+            }
+            .let { postfixExpression.addAll(it) }
+
+        return postfixExpression
+    }


점점 허리가 옆으로 휘어지는 코드들..ㅠㅠ

JiwonDev · 2023-08-14T17:39:35Z

src/test/kotlin/step2/application/CalculateUseCaseTest.kt

+class CalculateUseCaseTest : FreeSpec({
+
+    val calculateUseCase = CalculateUseCase(
+        expressionTokenizer = RegexExpressionTokenizer(),
+        expressionConvertor = PostfixExpressionConvertor(),
+        expressionCalculator = PostfixExpressionCalculator(),
+    )
+
+    "기본적인 사칙연산을 수행할 수 있어야 한다" - {
+        "숫자 하나만 입력" {
+            val result = shouldNotThrowAny { calculateUseCase.calculate("5") }
+
+            result shouldBe 5.0
+        }


적혀있는 테스트를 모두 통과하는 시점에서 바로 커밋하고 PR 날려봅니다 😀

Hyeon9mak

안녕하세요 지원님, 리뷰가 많이 지연되는 것 같아 먼저 코멘트 남깁니다.
각 유지보수 상황을 예측하며 책임을 나누신 모습이 인상적이에요 👍
아쉬운 점은 내부 구현이 많이 복잡해진거 같아요.
객체지향 생활체조를 되새기면서 메서드 분리, 확장 함수를 통해 복잡도를 낮춰보면 어떨까요?

Hyeon9mak · 2023-08-16T08:30:00Z

src/main/kotlin/step2/Step2Application.kt

@@ -0,0 +1,23 @@
+package step2


step1, step2 보다 현재 도메인에 걸맞는 패키지 명칭을 지어주는게 더 좋겠어요.
동료 개발자가 지원님 프로젝트를 열어봤을 때 의도를 바로 파악할 수 있게 되겠죠?

아 앞에 다른 리뷰에서도 봤었는데, 이게 상위 step1, step2 패키지에도 해당되는 말이었군요.
넵 의미있는 이름으로 변경하겠습니다 👍 세세한 리뷰 감사합니다!

Hyeon9mak · 2023-08-16T08:30:30Z

src/main/kotlin/step2/Step2Application.kt

+        // TODO main 구현
+        println("TEST: ${calculateUseCase.calculate("100/2+5")}")
+    }


동료 개발자가 '왜 주석이 있지?' 하고 갸우뚱 할 수 있겠어요.
불필요한 주석은 제거해주세요.

갑자기 println이 있으면 이상하게 생각할까봐 그랬던건데, 생각해보니 TODO 주석이 더 이상하네요. 제거하도록 하겠습니다 👍

Hyeon9mak · 2023-08-16T08:34:45Z

src/main/kotlin/step2/Step2Application.kt

+fun main(args: Array<String>) {
+    Step2Application().main(args)
+}


최상위 수준의 main 에서 Step2Application 의 main 을 다시 호출하신 이유가 있을까요?

Kotlin 1.3 이후 버전부터는 Array<String> 매개 변수가 생략될 수 있어요!

생각해보니, 자바가 아닌 코틀린이면 아예 main class를 만들 필요가 없었네요.
리뷰 반영해서 그냥 fun main() { .. } 하나만 남겨두도록 하겠습니다 👍

최상위 수준의 main 에서 Step2Application 의 main 을 다시 호출하신 이유가 있을까요?

kotlin에서 앱을 실행시키는 main함수를 사용하려면, top-level 로 만들거나 아래와 같이 해야합니다.
이렇게 하는거보단, 지금처럼 하는게 더 낫지 않을까 싶어서 main을 2개로 만들었습니다 😀

class Step2Application { companion object { @JvmStatic fun main(args: Array<String>) {...} } }

Hyeon9mak · 2023-08-16T08:37:41Z

src/main/kotlin/step2/entity/Expression.kt

@@ -0,0 +1,15 @@
+package step2.entity


엔티티는 Identifier 를 통해 프로퍼티들의 변화를 추적할 수 있는 객체를 의미한다고 생각해요.
현재 Expression, ExpressionToken, RawToken 에게 Identifier 와 상태변화가 존재하지 않는다면 이들은 VO 에 가까운 존재 아닐까요? 🤔

식별자와 상태변화가 존재하지않으면 VO에 가까운 존재 라는 말을 이해하지 못했어요. 혹시 조금만 더 설명해주실수 있을까요? 😀

처음에는 단순한 형태였다가 계속 수정되면서 필요한 시점에 생겨도 괜찮다고 생각합니다.
아니면 service에서 동작을 처리하는게 아니라, entity 자체에서 메시지를 주고받아서 동작하게 만들어라는 의미이신걸까요?

제가 너무 많은 단어를 생략하고 코멘트를 남겼네요 😱 🙏
entity 패키지 명칭에 대한 코멘트였어요.
저는 entity = id + vo 라고 생각하는데, 현재 entity 패키지 내부 클래스들은 모두 id 가 없어보여서 해당 코멘트를 남겼어요 🖊️

Hyeon9mak · 2023-08-16T08:38:26Z

src/main/kotlin/step2/entity/Expression.kt

+    companion object {
+        val NUMBER_REGEX: Regex = "([1-9][0-9]*)".toRegex()
+        val OPERATOR_REGEX: Regex = "[+\\-*/]".toRegex()
+        val TOKEN_REGEX: Regex = "$NUMBER_REGEX|$OPERATOR_REGEX".toRegex()
+        val EXPRESSION_REGEX: Regex = "$NUMBER_REGEX($OPERATOR_REGEX$NUMBER_REGEX)*".toRegex()
+    }


상수의 나열.. 그렇다면 혹시 enum class 는 어떨까요?

Expression. NUMBER_REGEX 이런식으로 사용하고 싶어서 요렇게 만들었습니다.
enum도 나쁘지않네요. 바꾸도록 하겠습니다 😀

Hyeon9mak · 2023-08-16T08:53:31Z

src/main/kotlin/step2/entity/ExpressionToken.kt

+data class NumberToken(val value: Double) : ExpressionToken {
+    companion object {
+        fun fromRawTokenOrNull(raw: RawToken): NumberToken? {
+            return raw.value.toDoubleOrNull()
+                ?.let(::NumberToken)
+        }
+    }
+}


data class 와 value class 의 차이는 무엇일까요?

이거 전에 찾아보고 정리해둔 내용인데, TMI니 읽지 않으셔도 됩니다.
읽어보면 좋은 블로그 Value Classes in Kotlin: Good-Bye Type Aliases?!

data class 는 개발자를 위한 코틀린 언어 자체의 편의 문법이라고 보면됩니다. 코틀린 컴파일러가 아래와 같이 만들어줍니다.

// 참고로 아래 메서드를 제공해주기 위해서, data class 는 최소 1개 이상의 필드가 있어야 컴파일 됩니다. .equlas() // 모든 필드를 equals로 하는 메서드. 참고로 코틀린은 == 연산자의 오버로딩으로 사용됩니다. .hashCode() // 모든 필드의 hashcode를 말아 하나의 hashcode로 만들어냅니다. .component1(), 2(), 3()... // 코틀린 객체 구조분해에 사용됩니다. .copy() // 얕은 복사 메서드

value class 는 조금 다릅니다. 컴파일 시점에 클래스 할당 없이, 내부의 값 자체가 바로 사용되도록 인라인 최적화됩니다.
JVM에서 사용하기 위해선, 반드시 @JvmInline 어노테이션을 붙여야합니다. 아니면 컴파일이 되지 않습니다.

바이트코드에선 Pair<>같은 primitvie type는 존재하지 않습니다. 그래서 상수(val)인 1개의 내부 필드만 가질 수 있습니다.

대신 init, 내부 메서드사용은 가능한데, 이들은 바이트코드상에서 static method 으로 선언됩니다.

@JvmInline value class Password(private val s: String)

🤨 근데 value class 만 써도 알아서 동작하게 해주지, 왜 value 클래스에만 사용되는 @JvmInline을 개발자가 직접 붙여야할까요? 코틀린스럽지 않잖아요.

🧑🏻‍💻 코틀린 value-classes 에 대한 디자인 및 의도

value class는 JVM 전용 기능이 아닙니다. 코틀린은 기본적으로 멀티 플랫폼을 지향하기에 내부 동작이 달라질 수 있습니다.

자바는 primitive type과 wrapper type이 별도로 존재합니다. 명시적으로 클래스가 아닌 상수임을 개발자에게 표시하고 싶었습니다.

primitive 와 wrapper 타입을 하나로 합치는 Project Valhalla가 JVM에 적용된 후 value class 자체가 변경될 것을 고려해서 코틀린 팀은 어노테이션으로 유연하게 컴파일 되도록 만들어뒀습니다.

🧐 근데 듣고보니까 value class 엄청난거같은데, 모든걸 value class로 도배해버리는건 어떨까요?
네 괜찮은 아이디어이긴하지만.. 아직까지 아쉬운점이 있긴합니다.

value class는 하나의 상수 값만 가질 수 있습니다. 확장성이 떨어집니다. 참고로 이는 코틀린이 아닌 JVM의 한계입니다.

value class는 JVM 레벨에서 래핑된 형태로 보이지 않습니다. 리플랙션등을 활용하기 어렵습니다

value class는 코틀린에 내장된 직렬화가 아닌 타 라이브러리를 사용할 때 예상하지 못한 동작을 보일 수 있습니다. 실제 Kotlin 이슈사례

잘 사용하면 좋긴하지만 굳이 필요하지 않은 곳에 적용할 필요가 있을까.. 고민되긴합니다.

NumberToken 이 하나 이상의 프로퍼티를 갖게 될지, 2단계에서 리플렉션이나 타 라이브러리를 사용할지 고민해보면 되겠네요!

그나저나 지원님 역시 하나를 공부할 때 엄청 깊게까지 공부하시네요.. 👍 👍

Hyeon9mak · 2023-08-16T08:56:53Z

src/main/kotlin/step2/service/ExpressionCalculator.kt

+    override fun calculate(expression: Expression): Double {
+        val stack: ArrayDeque<ExpressionToken> = expression.tokens
+            .fold(initial = ArrayDeque()) { stack: ArrayDeque<ExpressionToken>, token: ExpressionToken ->
+                when (token) {
+                    is NumberToken -> stack.apply { addLast(token) }
+                    is OperatorToken -> {
+                        val number2: ExpressionToken? = stack.removeLastOrNull()
+                        val number1: ExpressionToken? = stack.removeLastOrNull()
+                        val result: Double = this.evaluate(operator = token, number1 = number1, number2 = number2)
+                        stack.apply { addLast(NumberToken(result)) }
+                    }
+                }
+            }
+
+        require(stack.size == 1) { "계산이 완료되지 않은 표현식입니다." }
+
+        val result: NumberToken = stack.removeLastOrNull() as? NumberToken
+            ?: throw IllegalArgumentException("계산이 완료되지 않은 표현식입니다.")
+
+        return result.value
+    }


객체지향 생활체조는 이번 스터디에서 절대적인 기준이에요 😱
메서드로 분리하면서 복잡도를 낮춰보면 어떨까요?

객체지향 생활체조는 이번 스터디에서 절대적인 기준이에요 😱

핫 그렇군요..! 코드를 뜯어고치도록 하겠습니다 😀

한 메서드내에 추상화 수준을 동일하게 맞추고 싶었는데, calculate 안에 또 subCalculate 느낌으로 나누다보니 너무 과하게 추상화한 거 같아 고민되더라구요. 절대적인 기준이니 연습할 겸 메서드로 나눠보겠습니다!

"과한 추상화 == 알아보기 힘든 코드" 로 이해했어요.
현재 코드는 집중해서 자세히 살펴보면 직관적이게 잘 풀어져있지만,
하나의 메서드에 몰려있다보니 indent 가 깊어지고 읽기 두려운 코드가 된 것 같아요.
간단하게 private 메서드로만 분리해도 두려움이 사라질 것 같아요!

Hyeon9mak · 2023-08-16T08:59:57Z

src/main/kotlin/step2/service/ExpressionConvertor.kt

+    private fun toExpressionToken(rawToken: RawToken): ExpressionToken {
+        if (rawToken.value.matches(Expression.OPERATOR_REGEX)) {
+            return OperatorToken.fromRawTokenOrNull(rawToken)
+                ?: throw IllegalArgumentException("연산자로 변환할 수 없는 토큰입니다 (token=$rawToken)")
+        }
+
+        if (rawToken.value.matches(Expression.NUMBER_REGEX)) {
+            return NumberToken.fromRawTokenOrNull(rawToken)
+                ?: throw IllegalArgumentException("숫자로 변환 할 수 없는 토큰입니다 (token=$rawToken)")
+        }


"특정 문자('a', 'b', 'c')에 대해서는 연산자 토큰으로 변환될 수 있게끔 해주세요. 나머지는 잘못되었다는 메세지를 반환해주세요." 라는 요구사항을 전달 받았을 때, 지원님 동료 개발자는 OperatorToken.kt 와 ExpressionConvertor.kt 중 어떤 파일을 먼저 열어볼까요? 🤔

ExpressionConvertor 가 아니라 OperatorToken 이 메서드를 들고 있는게 더 나을수도 있겠네요.

지원님 동료 개발자는 OperatorToken.kt 와 ExpressionConvertor.kt 중 어떤 파일을 먼저 열어볼까요? 🤔

요걸 다시한번 생각해보면서, 리팩토링 해보도록 하겠습니다. 😀

cool~ 물론 사람마다 느껴지는 바가 다르기 때문에 '생각해보니까 ExpressionConvertor.kt 볼거 같은데?' 라면 패스하셔도 됩니다 😄 👍

JeremyShin

지원님 제가 얼른 2단계 끝내구 바로 리뷰드릴게요~!
3단계 먼저 하고계셔요 ㅠㅠ 늦어서 죄송함다

JiwonDev · 2023-08-16T12:54:22Z

리뷰가 막혀서 3단계를 못하고 있던건 아니고, 제가 게을러서..
갓윤철님 감사합니다 👍

JeremyShin · 2023-08-16T13:27:11Z

src/test/kotlin/step2/application/CalculateUseCaseTest.kt

+        }
+    }
+
+    "계산 순서는 사칙연산 규칙을 따르지 않고, 입력 값 순서대로 계산한다" - {


테스트코드에서 요구사항을 적어 검증하는군요 테스트 코드 작성 방법 .. 메모 ..

이와 유사하게 "기획자가 내 테스트코드를 볼때 이해할 수 있을까?" 관점으로 제목을 작성하려고 노력하기도 합니다ㅎ

feat: step2 문자열 계산기 구현

2cf702d

JiwonDev requested a review from JeremyShin August 14, 2023 17:25

JiwonDev self-assigned this Aug 14, 2023

JiwonDev commented Aug 14, 2023

View reviewed changes

Hyeon9mak reviewed Aug 16, 2023

View reviewed changes

JeremyShin reviewed Aug 16, 2023

View reviewed changes

JeremyShin approved these changes Aug 17, 2023

View reviewed changes

JiwonDev added 2 commits August 21, 2023 12:12

feat: [step2] 패키지 명 변경

b4699fc

feat: [step2] calculator 리팩토링 및 의미있는 이름으로 클래스명 변경

3e293f0

JiwonDev merged commit d65fc0f into mission-study-to-finish-in-15-days:jiwondev Aug 21, 2023

Hyeon9mak mentioned this pull request Aug 22, 2023

Step3 자동차 경주 #44

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: step2 문자열 계산기 구현 #26

feat: step2 문자열 계산기 구현 #26

JiwonDev commented Aug 14, 2023 •

edited

Loading

JiwonDev Aug 14, 2023

JiwonDev Aug 14, 2023

JiwonDev Aug 14, 2023

Hyeon9mak left a comment

Hyeon9mak Aug 16, 2023

JiwonDev Aug 16, 2023

Hyeon9mak Aug 16, 2023

JiwonDev Aug 16, 2023

Hyeon9mak Aug 16, 2023

Hyeon9mak Aug 16, 2023

JiwonDev Aug 16, 2023

Hyeon9mak Aug 16, 2023

JiwonDev Aug 16, 2023 •

edited

Loading

Hyeon9mak Aug 17, 2023 •

edited

Loading

Hyeon9mak Aug 16, 2023

JiwonDev Aug 16, 2023

Hyeon9mak Aug 16, 2023

JiwonDev Aug 16, 2023

Hyeon9mak Aug 17, 2023

Hyeon9mak Aug 17, 2023

Hyeon9mak Aug 16, 2023

JiwonDev Aug 16, 2023 •

edited

Loading

Hyeon9mak Aug 17, 2023

Hyeon9mak Aug 16, 2023

JiwonDev Aug 16, 2023

Hyeon9mak Aug 17, 2023 •

edited

Loading

JeremyShin left a comment

JiwonDev commented Aug 16, 2023

JeremyShin Aug 16, 2023

junuuuuuu Aug 17, 2023

feat: step2 문자열 계산기 구현 #26

feat: step2 문자열 계산기 구현 #26

Conversation

JiwonDev commented Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hyeon9mak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiwonDev Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

Hyeon9mak Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiwonDev Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hyeon9mak Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

JeremyShin left a comment

Choose a reason for hiding this comment

JiwonDev commented Aug 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiwonDev commented Aug 14, 2023 •

edited

Loading

JiwonDev Aug 16, 2023 •

edited

Loading

Hyeon9mak Aug 17, 2023 •

edited

Loading

JiwonDev Aug 16, 2023 •

edited

Loading

Hyeon9mak Aug 17, 2023 •

edited

Loading