[BE 이연호] 1주차 과제 제출합니다. #1111
[BE 이연호] 1주차 과제 제출합니다. #1111
Conversation
…alysis # 프로젝트의 기능 요구사항 및 기능 목록 초안 작성 (문제 핵심 파악 포함)
# feat(#1): MVC & TDD - Store user input in the model - Model: 사용자의 입력값을 생성자를 통해서 저장 # Model: Save user input via constructor - ModelTest: 사용자의 입력값을 기본 구분자(",:")로 분리 후 모델에 저장한 뒤, 이를 꺼냈을 때 값이 동일한지 테스트 # ModelTest: Split user input using default delimiters (",:"); # store it in the model and verify retrieved values match the original
# 기능목록 업데이트 - MVC & TDD - use Service to save user input into the Model # MVC & TDD - Service를 이용해서 모델에 사용자의 입력값을 저장
# feat(#2): Use Service to store user input into the model - 싱글톤을 이용해서, 입력값인 문자열에 기본 구분자들(쉼표(,)와 콜론(:))이 포함되어 있는지 체크 # Using a singleton, check if the input string contains the default delimiters (comma ',' and colon ':')
# feat(#2): Use Service to store user input into the model - 싱글톤을 이용해서, 입력값인 문자열에 "//"와 "\n" 사이의 커스텀 구분자가 있는지 체크 # Using a singleton, check if the input string contains a custom delimiter between "//" and "\n"
# docs(#2): update feature list - 서비스, 컨트롤러, 뷰의 기능목록을 업데이트 # Updated the feature list of Service, Controller, and View
# refactor(#2): move CalculatorService to a new package location - 변경된 패키지 위치: calculator.service.CalculatorService # Updated package location: calculator.service.CalculatorService
# feat(#2): complete MVC structure - 나머지 MVC 구조 제작 후, "커스텀_구분자_사용" 테스트 케이스 통과 # After completing the remaining MVC components, the "custom_delimiter_usage" test case passed
# docs(#2): update feature list – add test case to ensure only positive input values are accepted - 기능 목록 추가 사항: "입력값이 양수만 입력이 가능하도록 테스트 케이스 처리" # Added feature list item: test case ensuring only positive numbers can be entered
# feat(#2): handle test case to allow only positive input values - "예외_테스트()" 테스트 케이스를 통해, 입력값이 양수만 입력이 가능하도록 테스트 케이스 처리 # Through the "exception_test()" test case, ensure that only positive input values are allowed
…스 처리 # docs(#2): update feature list - add test case allowing both default and custom delimiters in user input - 기능목록 업데이트: 사용자의 입력값에서 기본 구분자와 커스텀 구분자를 함께 사용이 가능하도록 테스트 케이스 처리 # Updated feature list: test case ensuring both default and custom delimiters can be used together in user input
# feat(#2): add test case to allow both default and custom delimiters in user input - As-Is: 기존에는 사용자의 입력값 중에 커스텀 구분자와 기본 구분자를 함께 사용할 수 없었음. # As-Is: User input could not contain both custom and default delimiters. - To-Be: 사용자의 입력값에서 기본 구분자와 커스텀 구분자를 함께 사용이 가능하도록 테스트 케이스 처리 # To-Be: Added a test case to ensure that user input supports both default and custom delimiters.
# docs(#2): update feature list – actually use Model to store and retrieve a list of numbers separated by delimiters - 사용자의 입력값을 Model을 사용해서 구분자로 정리된 숫자 목록을 저장 후, 꺼내어 사용 # Used the Model to store the list of numbers separated by delimiters and retrieve them when needed
# feat(#2): actually use Model to store and retrieve a list of numbers separated by delimiters - As-Is: 기존에는 사용자의 입력값을 모델에 저장하지 않음. # As-Is: The user input was not stored in the model. - To-Be: 사용자의 입력값을 Model을 사용해서 구분자로 정리된 숫자 목록을 저장 후, 꺼내어 사용 # To-Be: Use the Model to store the list of numbers separated by delimiters and retrieve them when needed.
KennyKim33
left a comment
KennyKim33
left a comment
There was a problem hiding this comment.
코드 전체적으로 잘 보았습니다.
특히 TDD 방식으로 테스트를 먼저 작성하고, 이를 기반으로 기능을 구현해 나가신 점이 매우 인상 깊었습니다.
TDD 경험이 많지 않은 저에게도 좋은 학습 예시가 되어서 많이 배웠습니다.
한 가지 커밋 메시지 관련해서 궁금한 점이 있습니다.
모든 커밋이 feat(#2) 형태로 작성되어 있던데, 혹시 특정 이슈 번호와 연결하기 위한 용도였을까요?
추가로 제안드리고 싶은 부분은,
feat(#2)처럼 모든 커밋을 동일한 태그로 작성하기보다는,
feat(기능명) 혹은 feat(관련 패키지명)처럼 구체적인 의미를 드러내도 좋을 것 같습니다.
이렇게 하면 커밋 히스토리를 나중에 확인할 때도
“어떤 부분에 대한 기능 추가였는지”를 더 빠르게 파악할 수 있다는 장점이 있을 것 같습니다.
| public static String extractCustomDelimiter(String userInput) { | ||
| String startDelimiter = "//"; | ||
| String endDelimiter = "\\n"; | ||
|
|
||
| Integer startIndex = userInput.indexOf(startDelimiter) + startDelimiter.length(); | ||
| Integer endIndex = userInput.indexOf(endDelimiter); | ||
| if (endIndex == -1) { | ||
| // 실제 줄바꿈 문자를 인식하도록 처리 | ||
| endDelimiter = "\n"; | ||
| endIndex = userInput.indexOf(endDelimiter); | ||
| } | ||
|
|
||
| return (startIndex != -1 && endIndex != -1 && startIndex < endIndex) | ||
| ? userInput.substring(startIndex, endIndex) : ""; | ||
| } | ||
|
|
There was a problem hiding this comment.
startDelimiter와 endDelimiter는 매직 스트링으로 보이는데, 상수로 분리해두면 의미가 명확해지고 재사용성이 높아질 것 같습니다.
예: private static final String START_DELIMITER = "//";
삼항 연산자를 활용한 부분은 간결하지만, 조건이 길어지면서 가독성이 조금 떨어질 수 있습니다.
조건식을 별도의 변수에 담거나 메서드로 분리하면 읽기가 더 편해질 것 같습니다.
| // 숫자 유효성 검사: 숫자가 아니거나 음수인 경우 예외 발생 | ||
| public static void validateNumber(String validateNumber) { | ||
| try { | ||
| Integer number = Integer.parseInt(validateNumber); | ||
| if (number < 0) { | ||
| throw new IllegalArgumentException("양수만 입력 가능합니다." + validateNumber); | ||
| } | ||
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException("숫자가 아닙니다. " + validateNumber); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
사용자에게 입력을 받는 부분은 enum을 사용해서 처리를 해주셨던 거 같은데, 예외를 던지는 메시지 같은 경우도 상수로 처리하거나 enum을 사용하는 건 어땠을까요?
개인적으로는 통일성있는 코드 구조를 선호하다보니 읽다가 궁금증이 생겼습니다. 의도하신게 있으실까요?
| public static List<String> splitByDelimiter(String userInput, String delimiter) { | ||
| return Arrays.stream(userInput.split(delimiter)) | ||
| .map(String::trim) // 공백 제거 | ||
| .peek(CalculatorService::validateNumber) // 숫자 유효성 검사 |
There was a problem hiding this comment.
제가 알기론 stream의 peek 은 디버깅을 사용할 때 유용하게 쓰는 메서드로 알고 있습니다.
현재처럼 중요한 검증 로직을 peek 안에 넣게 되면 의도가 잘 드러나지 않아,
나중에 코드를 읽을 때 혼동을 줄 수 있을 것 같습니다.
오히려 validate와 같이 중요한 로직을 검증하는 것이라면, 중간에 빼서 따로 사용하는게 더 좋지 않았을까 하는 생각이 듭니다.
| // Model 단에 사용자의 입력값을 저장 | ||
| calculatorModel = new CalculatorModel(splitByDelimiter(content, delimiter)); | ||
|
|
||
| // 숫자로 변환한 후 합계 계산 | ||
| return numbers.stream() | ||
| return calculatorModel.getUserInputList().stream() |
There was a problem hiding this comment.
사용자 입력값을 CalculatorModel에 한 번 저장한 뒤,
다시 그 값을 꺼내서 계산하는 흐름이 다소 번거롭게 느껴졌습니다.
혹시 이렇게 구성하신 데에 특별한 의도가 있으실까요?
입력 문자열로부터 CalculatorModel을 생성하는 단계와,
생성된 CalculatorModel을 파라미터로 받아 계산하는 로직을 각각 메서드로 분리하면
역할이 더 분명해지고, 테스트나 재사용 관점에서도 이점이 있을 것 같다는 생각이 들었습니다.
| return numbers.stream() | ||
| return calculatorModel.getUserInputList().stream() | ||
| .map(Integer::parseInt) // 문자열을 Integer로 변환 | ||
| .reduce(0, Integer::sum); // 합계 |
There was a problem hiding this comment.
제가 알기로는 .map(Integer::parseInt)는 Stream를 만들어서
sum()를 사용할 수 없는 구조라서 reduce를 쓰신 것 같습니다.
Integer 스트림보다는 mapToInt(Integer::parseInt)를 활용해
primitive IntStream으로 변환하면 .sum()을 바로 사용할 수 있어
더 간결하고 박싱 오버헤드도 줄일 수 있을 것 같습니다.
2 participants
1주차 과제: 구분자로 자른 문자열의 숫자값 덧셈