[Step1] 계산기 #3
Merged
[Step1] 계산기 #3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Delimiter를 기준으로 문자 토큰을 잘라서 리스트로 반환
galcyurio
approved these changes
Jul 24, 2021
Comment on lines
+27
to
+53
| @RunWith(Parameterized::class) | ||
| class OperatorFindParameterTest( | ||
| private val operatorToken: String, | ||
| private val expectedOperator: Operator, | ||
| ) { | ||
| companion object { | ||
| @JvmStatic | ||
| @Parameters(name = "{0} 토큰으로 {1} 연산자를 찾는다") | ||
| fun tokenAndOperators(): Collection<Array<Any>> { | ||
| return listOf( | ||
| arrayOf("+", Operator.PLUS), | ||
| arrayOf("-", Operator.MINUS), | ||
| arrayOf("*", Operator.MULTIPLY), | ||
| arrayOf("/", Operator.DIVIDE), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `입력 문자에 맞는 연산자를 찾는다`() { | ||
| // when | ||
| val actualOperator = Operator.find(operatorToken) | ||
|
|
||
| // then | ||
| assertThat(actualOperator).isEqualTo(expectedOperator) | ||
| } | ||
| } |
There was a problem hiding this comment.
JUnit4에서의 하나의 방법이 될 수 있을 것 같네요. 👍
이번 미션에서는 domain 모듈로 분리를 해주셨는데 domain 모듈에서 어차피 안드로이드에 의존성이 없는 코드들만 위치하니 JUnit5를 쓰는 방식도 좋을 것 같습니다.
Comment on lines
+2
to
+3
| id 'com.android.library' | ||
| id 'kotlin-android' |
There was a problem hiding this comment.
domain 모듈은 순수 코틀린 모듈로 만드는게 좋습니다.
Comment on lines
+35
to
+40
| // when | ||
| val expression = "+ 1 * 2" | ||
|
|
||
| // then | ||
| assertThrows(IllegalArgumentException::class.java) { StringCalculator.calculate(expression) } | ||
| .also { assertThat(it).hasMessageThat().contains("수식의 숫자 자리에 숫자가 아닌 문자 (+)가 존재합니다.") } |
There was a problem hiding this comment.
또, Truth에서는 assertJ의 assertThatThrownBy{} 메서드가 없어서 어떻게 구현 하면 좋을 지 생각을 많이 해봤습니다. 스텍 오버플로 답글 중에, 구글 truth 만든 사람이 해당 메서드가 필요 없다고 생각했다는 말도 있네요.
일단 제 경험을 말씀드리자면 assertj를 사용할 때에는 assertThatThrownBy() 함수보다는 catchThrowable() 함수를 사용했습니다.
그 이유는 BDD 형태의 테스트 코드를 쓸 때 assertThatThrownBy() 함수를 쓰면 when과 then이 합쳐져버립니다.
때문에 명시적으로 'x를 했을 때 x라는 에러가 일어나야 한다.'라는 식으로 코드를 작성하기 위해서 catchThrowable()을 활용했습니다.
마찬가지로 Truth에서도 catchThrowable()은 없지만 코틀린의 runCatching() 함수를 활용하여 비슷하게 작성할 수 있습니다.
@Test
fun `숫자 자리에 다른 문자가 들어가면 Exception`() {
// given
val expression = "+ 1 * 2"
// when
val actual = runCatching { StringCalculator.calculate(expression) }.exceptionOrNull()
// then
assertThat(actual).hasMessageThat().contains("수식의 숫자 자리에 숫자가 아닌 문자 (+)가 존재합니다.")
}There was a problem hiding this comment.
이와는 별개로 Exception을 던지는 방식보다 null을 반환하거나 실행 결과를 나타내는 sealed class를 반환하는 방식도 고려해보면 좋겠습니다.
예를 들면, calculate() 함수를 호출했을 때 수식이 올바르지 않아 결과를 줄 수 없을 경우에 Exception을 던지는 대신 null을 반환할 수도 있겠죠?
코틀린에서 공식적으로 이런 방식을 위해 지원하는 것이 runCatching() 함수와 Result<T> 클래스입니다.
ptm0304
added a commit
to ptm0304/android-calculator-mvc
that referenced
this pull request
Jul 18, 2022
- 2중 if 문 when으로 처리 - 한줄로 끝나지 않는 if else 문 안드로이드 공식 컨벤션에 따라 생략 x
ptm0304
added a commit
to ptm0304/android-calculator-mvc
that referenced
this pull request
Jul 18, 2022
- 버그 수정 ㄴ 입력된 피연산자가 없을때 아무 연산자 버튼 클릭시 "-"가 나오는 버그 ㄴ 테스트용 setDisplayedText 함수가 white space 로 인해 정상 동작하지 않는 문제
namjackson
pushed a commit
that referenced
this pull request
Jul 20, 2022
* 3단계 피드백 반영 - domain 테스트 코드 domain 모듈로 이동 - Calculator, Operator 책임 분리 * step3 개선: unit test 코드 정리 및 개선 - test case 네이밍 개선 - jUnit5 적용 * feat: 계산기 UI # 1 - UI 관련 기본 베이스 구조 구성 (InputController, InputHandler, DataBinding) - Domain models (ExpressionElement, Operand, Operator, Symbol) * 개선: app 모듈 jUnit5 적용 * step4 feat: UI 테스트 케이스 작성 - 입력된 피연산자가 없을 때, 사용자가 피연산자 0 ~ 9 버튼을 누르면 화면에 해당 숫자가 화면에 보여야 한다. - 입력된 피연산자가 있을 때, 기존 숫자 뒤에 해당 숫자가 화면에 보여야 한다. 예를 들면, 8이 입력되어 있을 때 9를 입력하면 89가 보여야 한다. - 입력된 피연산자가 없을 때, 사용자가 연산자 +, -, ×, ÷ 버튼을 누르면 화면에 아무런 변화가 없어야 한다 - 입력된 피연산자가 있을 때, 사용자가 연산자 +, -, ×, ÷ 버튼을 누르면 해당 기호가 화면에 보여야 한다. - 입력된 수식이 없을 때, 사용자가 지우기 버튼을 누르면 화면에 아무런 변화가 없어야 한다. - 입력된 수식이 있을 때, 사용자가 지우기 버튼을 누르면 수식에 마지막으로 입력된 연산자 또는 피연산자가 지워져야 한다. - 입력된 수신이 완전할 때, 사용자가 = 버튼을 누르면 입력된 수식의 결과가 화면에 보여야 한다. * step4 feat: 나머지 기능 구현 - Input 모델 따로 정의 - 기능 요구사항에 명시된 케이스 처리 - InputControllerTest 추가 - 곱하기, 나누기 캐릭터 변경 ㄴ "/" -> "÷", "*" -> "×" * InputControllerTest 네이밍 개선 * refactor: step4 피드백 반영 - Input 관련 네이밍 변경 ㄴ Input -> UserInputAction ㄴ InputController -> UserInputActionProcessor ㄴ InputHandler -> UserInputActionReceiver - RegexUtils > Operator 관련 정규식을 상수가 아닌 Operator Enum에서 문자열을 받는 방식으로 변경 - 버튼마다 각각의 Input을 지정해주지 않고, view를 param으로 받아서 처리하도록 수정 - Input 관련 모델 제거 및 기존 Operand, Operator 모델과 병합 * refactor: step4 피드백 반영 #2 - UserInputAction 네이밍 변경 -> ExpressToken * refactor: step4 피드백 반영 #3 - 2중 if 문 when으로 처리 - 한줄로 끝나지 않는 if else 문 안드로이드 공식 컨벤션에 따라 생략 x * refactor: step4 피드백 반영 #4 - RegexUtils에 joinToString 활용 * refactor: step4 피드백 반영 #5 - ExpressionToken > getFromValue 에서 EnumClass.values().find{..} 활용하는 방식으로 수정 * refactor: step4 개선 - 음수 지원 기능 구현 - 숫자 앞에 '-'가 오는걸 허용 - 마지막 인풋이 연산자 였다면 뒤에 "-"를 인풋으로 추가하는걸 허용 - 기존 마지막 인풋이 연산자라면 사용자가 연산자 버튼을 누르면 해당기호가 화면에 보여야한다는 요구사항에서 "-"는 제외 - 연산자 사이에는 white space를 넣어줌으로써 음수 "-"와 뺴기 "-"를 구분 * refactor: step4 개선 - 음수 지원 기능 구현 #2 - 테스트 케이스 음수 지원 스펙 변경에 맞게 수정 * refactor: step4 개선 - 음수 지원 기능 구현 #3 - 버그 수정 ㄴ 입력된 피연산자가 없을때 아무 연산자 버튼 클릭시 "-"가 나오는 버그 ㄴ 테스트용 setDisplayedText 함수가 white space 로 인해 정상 동작하지 않는 문제 * refactor: step4 피드백 반영 #6
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
domain을 말씀하신대로 멀티모듈로 나누어서 구현해보았습니다.
코드를 작성하다보니 처음 코틀린 강의에서 작성했던 미션과 거의 비슷하게 짠 것 같네요. 신기합니다.
Operand 쪽은 하다가 오버스펙이라고 생각해서 문서에서 지운 흔적을 남겼는데, 컴퓨터에서 저장을 안했는지 깃에 반영이 되지 않았네요..
Parameterized 테스트가 JUnit4 에서는 굉장히 불편하군요. 그래서 테스트 케이스 별로 클래스를 나눠서 했는데, 괜찮아 보이는 방법이었을까요? 또, Truth에서는 assertJ의 assertThatThrownBy{} 메서드가 없어서 어떻게 구현 하면 좋을 지 생각을 많이 해봤습니다. 스텍 오버플로 답글 중에, 구글 truth 만든 사람이 해당 메서드가 필요 없다고 생각했다는 말도 있네요.
https://stackoverflow.com/a/38866403/10113634
이번 리뷰 잘 부탁드립니다.