feat(calculator): 문자열 덧셈 계산기 구현 #758

all-cloudz · 2025-01-24T09:59:20Z

안녕하세요! 먼저 주어진 요구 사항을 해결할 수 있는 정도로만 구현을 해보았습니다.

인터페이스나 제네릭 등을 이용하여 조금 더 유연하고 확장 가능하게 코드를 작성하는 것을 고민하다가 시도해보았으나,
현재 과제의 내용에 비해 너무 과한 것을 하고 있는 것 같다는 생각이 들어 다시 코드를 작성하느라 첫 번째 코드 리뷰 요청이 늦어졌는데요.

리뷰어님께 피드백 받으며 코드를 점차 개선하는 방향이 저의 배움에 더 도움이 되지 않을까 싶어서 리뷰 요청 드립니다 :)
그리고 새해 복 많이 받으세요 😊

jkshin0602

안녕하세요. 이번 미션을 함께 진행할 신재권입니다. 👍
늦은 시간에도 미션 진행하시느라 고생 많으셨습니다.

몇가지 피드백 남겨드렸으니, 확인 부탁드려요!

src/main/kotlin/calculator/NonNegativeNumber.kt

jkshin0602 · 2025-01-24T16:50:22Z

src/main/kotlin/calculator/NonNegativeNumber.kt

+fun Int.toNonNegativeNumber() = NonNegativeNumber(this)
+
+fun String.toNonNegativeNumber() = this.toInt().toNonNegativeNumber()
+
+fun List<Int>.toNonNegativeNumbers() = map { it.toNonNegativeNumber() }


Int 클래스는 많은 곳에서 사용할텐데, 모든 Int 클래스가 해당 기능을 본다면, 불필요한 로직들이 많이 노출되는 것 같습니다.

정의하신 클래스 NonNegativeNumber만 활용해도 충분할 것 같습니다.

String, List도 동일하게 불필요한 노출이 생기는 것 같습니다.

개인적으로 Int, String, List 등에 대해서 확장 함수로 변환 로직을 유틸성 기능으로 제공하는 것이 이 경우에는 그렇게 어색하지 않다고 생각했는데요. 코틀린에서 toLong(), toString()와 같은 함수를 제공하기 때문에 그렇게 생각했던 것 같아요.

또한, 저는 클라이언트 함수에 스코프 함수 등을 이용하여 추가적인 연산을 수행하는 것보다 확장 함수를 이용하여 클라이언트가 사용하기에 자연스러운 코드 흐름을 제공하는 것이 더 낫다는 생각이 있는데요. (이는 자바에서는 정적 함수를 굳이 제공하는 게 아니라면 할 수 없었지만 코틀린이기에 확장 함수를 이용하여 편하게 할 수 있는 것 같기는 합니다.)

제가 이렇게 프로그래밍 하는 것이 불필요한 로직을 노출시킨다는 문제가 있다는 것을 알고 있지만 개인적으로는 큰 불편함이 아니라고 생각해서 그냥 이렇게 사용했었습니다! 혹시 제가 간과하고 있는 문제가 있는 것일까요?

일단 Int를 NonNegativeNumber로 바꾸는 것은 사실 필요하지 않는 함수라 ~~String을 NonNegativeNumber로 변환하는 것만 남겨두고 지우겠습니다 :)~~

더 곰곰히 생각해봤는데 이 경우에는 클라이언트 함수가 추가적인 연산을 수행하는 것이 그렇게 복잡하지는 않아서 말씀해주신대로 확장 함수를 사용해서 코드를 읽기 편해지는 이득보다 불필요한 메서드를 노출함으로서 생기는 손해가 더 큰 것 같아서 모두 지우겠습니다!

깊이 생각하시고 답변주시느라 고생하셨습니다. 👍

개인적인 의견은 다음과 같습니다.
다 같이 하는 프로젝트라면, 불필요한 로직을 노출시키는 것은 지양해야 한다고 생각합니다. 특히 의도대로 사용하지 않고 해당 클래스를 오용할 가능성이 있기 때문입니다.

사실 정답은 없습니다. 상황에 따라 맞추는 것을 추천드려요.

추가적으로 toLong(), toString() 같은 변환 메서는 범용적인 메서드라고 생각합니다.

개인적으로 특정 클래스로 변환 시키는 로직을 확장 함수로 정의 많이 하기도 합니다. 하지만 전체적으로 노출 시키는 것이 아닌 사용하는 곳 종류에 따라 companion을 활용하거나, 변경시키는 곳이 있는 파일에 private으로 확장 함수를 선언하는 편입니다.

jkshin0602 · 2025-01-24T16:54:37Z

src/main/kotlin/calculator/NonNegativeNumber.kt

+
+fun List<Int>.toNonNegativeNumbers() = map { it.toNonNegativeNumber() }
+
+fun List<NonNegativeNumber>.sum(): Int = this.sumOf { it.value }


확장 함수도 좋지만, 일급 컬렉션 또는 확장 함수 제거 후, list 부분에서 sumOf{ }를 호출하는 방향도 좋을 것 같습니다.

개인적으로 확장 함수는 변경할 수 없는 부분이나, 어려운 부분에 활용하는 편입니다. (라이브러리, 레거시 코드)
다운님은 주로 확장함수를 언제 활용하실까요? 👍

저는 개인적으로 코틀린에서 일급 컬렉션이 반드시 필요하다고 느껴지는 게 아니면 대체로 정의하지 않는 편입니다!

자바에서 일급 컬렉션을 사용했던 이유를 '(과거의 이야기 일 수 있지만) 자바에서는 주로 가변 컬렉션(ArrayList 등)을 사용하고, 컬렉션의 요소도 종종 가변의 가능성을 열어두었기 때문에 이를 안정적으로 관리하기 위해서' 라고 생각했는데요.

코틀린에서는 MutableList로 억지로 다운 캐스팅하지 않는다는 컨센서스만 팀이 공유한다면 불변 컬렉션을 유지하기가 쉽고, 불변 데이터 클래스를 이용한다면 컬렉션의 요소가 변경되는 것도 걱정할 필요가 없다고 생각했기 때문입니다.

그래서 이 경우에는 NonNegativeNumber를 정의하고, List에 필요한 계산은 확장 함수로 정의한 것이었는데 혹시 제 생각이 많이 어색하게 느껴지셨을까요? 제가 코틀린을 사용한지 한 달이 채 되지 않아서 아직 어떻게 사용하는 게 베스트 프랙티스인가에 대한 감이 많이 없는 상태인데요. 참고할 수 있는 자료나 책이 있다면 추천해주실 수 있을까요 😊

다운님이 말씀하신 방향은 일급 컬렉션의 장점 중 하나인 불변성을 말하는 것 같습니다. 물론 코틀린에서는 불변, 가변 자료구조가 구분되어 있어 안정적으로 관리가 가능합니다.

일급 컬렉션을 사용하는 이유가 불변성 외에 도메인 종속적인 자료구조, 상태나 행위를 한 곳에서 관리하는 것 등의 장점이 존재한다고 생각합니다. 코틀린에서 확장 함수로 대부분 커버가 될 것 같네요 👍

다운님의 생각에 동의가 됩니다! 하나 배워가겠습니다. 💯

저도 코틀린을 사용한지 1년 조금 넘은 것 같습니다. 다운님처럼 항상 베스트 프랙티스에 대한 고민이 많습니다.
저도 아직까지 완벽하게 해소되지는 않았지만, 코틀린을 공부할 때 읽었던 책인 이펙티브 코틀린을 추천합니다.
해당 책으로 어느정도 해소가 될 수 있을 것이라 생각해요! 😀

src/main/kotlin/calculator/InputString.kt

src/test/kotlin/calculator/CalculatorTest.kt

src/main/kotlin/calculator/ParsingService.kt

jkshin0602

궁금한 점이 있으시다면 언제든 물어봐주세요~👍

jkshin0602

피드백 잘 반영해주셨습니다. 고생 많으셨어요! 👍
추가로 코멘트 남겼습니다!
간단하게 생각 정리하시고, 요청 주시면 다음 단계로 진행할 수 있도록 하겠습니다! 👏

jkshin0602 · 2025-01-26T07:38:46Z

src/main/kotlin/calculator/NonNegativeNumber.kt

+fun Int.toNonNegativeNumber() = NonNegativeNumber(this)
+
+fun String.toNonNegativeNumber() = this.toInt().toNonNegativeNumber()
+
+fun List<Int>.toNonNegativeNumbers() = map { it.toNonNegativeNumber() }


깊이 생각하시고 답변주시느라 고생하셨습니다. 👍

개인적인 의견은 다음과 같습니다.
다 같이 하는 프로젝트라면, 불필요한 로직을 노출시키는 것은 지양해야 한다고 생각합니다. 특히 의도대로 사용하지 않고 해당 클래스를 오용할 가능성이 있기 때문입니다.

사실 정답은 없습니다. 상황에 따라 맞추는 것을 추천드려요.

jkshin0602 · 2025-01-26T07:41:15Z

src/main/kotlin/calculator/NonNegativeNumber.kt

+fun Int.toNonNegativeNumber() = NonNegativeNumber(this)
+
+fun String.toNonNegativeNumber() = this.toInt().toNonNegativeNumber()
+
+fun List<Int>.toNonNegativeNumbers() = map { it.toNonNegativeNumber() }


추가적으로 toLong(), toString() 같은 변환 메서는 범용적인 메서드라고 생각합니다.

개인적으로 특정 클래스로 변환 시키는 로직을 확장 함수로 정의 많이 하기도 합니다. 하지만 전체적으로 노출 시키는 것이 아닌 사용하는 곳 종류에 따라 companion을 활용하거나, 변경시키는 곳이 있는 파일에 private으로 확장 함수를 선언하는 편입니다.

jkshin0602 · 2025-01-26T07:50:48Z

src/main/kotlin/calculator/NonNegativeNumber.kt

+
+fun List<Int>.toNonNegativeNumbers() = map { it.toNonNegativeNumber() }
+
+fun List<NonNegativeNumber>.sum(): Int = this.sumOf { it.value }


다운님이 말씀하신 방향은 일급 컬렉션의 장점 중 하나인 불변성을 말하는 것 같습니다. 물론 코틀린에서는 불변, 가변 자료구조가 구분되어 있어 안정적으로 관리가 가능합니다.

일급 컬렉션을 사용하는 이유가 불변성 외에 도메인 종속적인 자료구조, 상태나 행위를 한 곳에서 관리하는 것 등의 장점이 존재한다고 생각합니다. 코틀린에서 확장 함수로 대부분 커버가 될 것 같네요 👍

다운님의 생각에 동의가 됩니다! 하나 배워가겠습니다. 💯

jkshin0602 · 2025-01-26T07:53:34Z

src/main/kotlin/calculator/NonNegativeNumber.kt

+
+fun List<Int>.toNonNegativeNumbers() = map { it.toNonNegativeNumber() }
+
+fun List<NonNegativeNumber>.sum(): Int = this.sumOf { it.value }


저도 코틀린을 사용한지 1년 조금 넘은 것 같습니다. 다운님처럼 항상 베스트 프랙티스에 대한 고민이 많습니다.
저도 아직까지 완벽하게 해소되지는 않았지만, 코틀린을 공부할 때 읽었던 책인 이펙티브 코틀린을 추천합니다.
해당 책으로 어느정도 해소가 될 수 있을 것이라 생각해요! 😀

jkshin0602 · 2025-01-26T07:56:02Z

src/main/kotlin/calculator/Delimiter.kt

+@JvmInline
+value class Delimiter private constructor(


value class 활용 👍

value class 사용 시 뭐가 좋을까요?

추가적으로 value class를 메서드 인자로 넘기면 mangling 이라는 것이 일어나는 데, 관련해서 한 번 찾아보시는 것도 좋을 것 같아요. 이것 때문에 실무에서 가끔 문제가 발생할 수 있습니다.

jkshin0602 · 2025-01-26T07:56:42Z

src/main/kotlin/calculator/InputString.kt

+        require(allowedFormat.matches(src)) {
+            "잘못된 Input String 입니다. ($src)"


require 👍

jkshin0602 · 2025-01-26T08:03:41Z

src/main/kotlin/calculator/Delimiter.kt

+fun MatchResult?.extractDelimiterRegex(): Regex? =
+    this?.groups
+        ?.get("delimiter")
+        ?.value
+        ?.let { Regex.escape(it) }
+        ?.toRegex()


excape 처리 👍

jkshin0602 · 2025-01-26T08:05:12Z

src/main/kotlin/calculator/NonNegativeNumber.kt

        }
    }
+
+    companion object {
+        const val LOWER_BOUND = 0


매직 넘버 제거 👍
LOWER_BOUND를 외부에서 사용하지 않는다면 private 으로 선언해도 좋을 것 같습니다.

jkshin0602 · 2025-01-26T08:17:31Z

src/test/kotlin/calculator/InputStringTest.kt

+    @Test
+    fun `기본 구분자와 숫자가 있는 정상 패턴`() = repeat(100) {
+        val target = RandomFixtureGenerator.generateBy(10, useCustomDelimiter = false)
+        assertThat(InputString.of(target).src).isEqualTo(target)
+    }
+
+    @Test
+    fun `커스텀 구분자와 숫자가 있는 정상 패턴`() = repeat(100) {
+        val target = RandomFixtureGenerator.generateBy(10, useCustomDelimiter = true)
+        assertThat(InputString.of(target).src).isEqualTo(target)
+    }


테스트 의도는 이해하였습니다 👍
만약 InputString.of() 로직이 오래걸리는 로직이라면 어떻게 될까요? 이를 어떻게 개선해볼 수 있을까요?

all-cloudz added 11 commits January 23, 2025 23:31

fix: 잘못된 테스트 코드 경로 수정

3cea6b0

docs: 요구 사항 정리

63a1652

feat(caculator): Delimiter 추가

551e0bc

feat(caculator): InputString 추가

5d6ab58

feat(caculator): NonNegativeNumber 추가

2c1f5e9

feat(caculator): ParsingService 추가

282c107

feat(caculator): Calculator 추가

5d7581d

style: 변수명 수정

b922ee8

refactor: ParsingService 클래스의 함수를 최상위 함수로 변경

51abeb4

refactor: 사용하지 않는 생성자 삭제

5edfa09

test: 테스트 케이스 추가

32b63b5

jkshin0602 reviewed Jan 24, 2025

View reviewed changes

jkshin0602 requested changes Jan 24, 2025

View reviewed changes

all-cloudz added 6 commits January 25, 2025 16:59

refactor: data class를 value class로 수정

d19e4c0

refactor: 확장 함수를 메서드로 변경

4828ac3

fix(calculator): Delimiter 추출 정규식에 escape 추가

585b886

refactor: 매직 넘버 상수화

e62dae7

refactor: 사용하지 않는 확장함수 제거

2434895

refactor: Regex에서 값을 추출하는 코드를 메서드 분리

5075b33

jkshin0602 requested changes Jan 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(calculator): 문자열 덧셈 계산기 구현 #758

feat(calculator): 문자열 덧셈 계산기 구현 #758

all-cloudz commented Jan 24, 2025

jkshin0602 left a comment

jkshin0602 Jan 24, 2025

all-cloudz Jan 25, 2025 •

edited

Loading

all-cloudz Jan 25, 2025 •

edited

Loading

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 24, 2025

all-cloudz Jan 25, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 left a comment •

edited

Loading

jkshin0602 left a comment

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025

jkshin0602 Jan 26, 2025


		fun List<Int>.toNonNegativeNumbers() = map { it.toNonNegativeNumber() }

		fun List<NonNegativeNumber>.sum(): Int = this.sumOf { it.value }

		require(allowedFormat.matches(src)) {
		"잘못된 Input String 입니다. ($src)"

feat(calculator): 문자열 덧셈 계산기 구현 #758

Are you sure you want to change the base?

feat(calculator): 문자열 덧셈 계산기 구현 #758

Conversation

all-cloudz commented Jan 24, 2025

jkshin0602 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

all-cloudz Jan 25, 2025 • edited Loading

Choose a reason for hiding this comment

all-cloudz Jan 25, 2025 • 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

jkshin0602 left a comment • edited Loading

Choose a reason for hiding this comment

jkshin0602 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

Choose a reason for hiding this comment

all-cloudz Jan 25, 2025 •

edited

Loading

all-cloudz Jan 25, 2025 •

edited

Loading

jkshin0602 left a comment •

edited

Loading