Ecosyste.ms: Timeline

Browse the timeline of events for every public repo on GitHub. Data updated hourly from GH Archive.

woowacourse-precourse/java-calculator-7

dye0p created a review on a pull request on woowacourse-precourse/java-calculator-7

View on GitHub

dye0p created a review comment on a pull request on woowacourse-precourse/java-calculator-7
> CalculateController에서 메서드 분리가 조금 과한 것 같다는 생각이 듭니다 `calculateSum`과 `displayResult` 메서드를 한번 더 분리해서, run 메서드만 봤을 때는 이해하기가 좋지만 협업하는 관점에서, 두 메서드는 다른 메서드를 한번 호출하는 역할만 수행하는데 로직을 깊게 이해하려면 한번 더 메서드를 타...

View on GitHub

dye0p created a review on a pull request on woowacourse-precourse/java-calculator-7

View on GitHub

aka-nick created a review comment on a pull request on woowacourse-precourse/java-calculator-7
타입이 String인 이상 어떠한 구문일거니까, `STATEMENT`를 제외하면 좀 더 간결하게 같은 의미를 전달할 수 있지 않을까요? 구문이 아닌 어떤, 구분자라든지.. 헤더라든지.. 하는 것들이 함께 정의되어있다면 경우가 다를 수도 있겠지만요!

View on GitHub

aka-nick created a review comment on a pull request on woowacourse-precourse/java-calculator-7
예외 메시지도 좋은 정보이기 때문에 정성들여 작성해주면 스택트레이스를 다 뒤지지 않아도 버그를 찾기 쉬워지고 도움이 되어요 👍

View on GitHub

aka-nick created a review comment on a pull request on woowacourse-precourse/java-calculator-7
입력 문자에 따라, 입력 문자의 문자셋에 따라, 한 문자를 표현할 수 있는 길이가 달라질 수 있다는 사실 알고 계셨나용? 🤗 입력 문자가 아주 제한되어 있는 것이 아니라면 문자 위치를 추측하는 것(만약 커스텀 구분자가 있으면 몇번째 자리에는 뭐가 있을거야)보다 헤더와 푸터의 위치를 찾고 그 사이의 문자를 찾아내는 것이 정확해요! 물론 길이체크를 따...

View on GitHub

aka-nick created a review comment on a pull request on woowacourse-precourse/java-calculator-7
아래의 @dradnats1012 님 의견과 비슷한 코멘트인데요. 메서드 시그니처(`리턴값 메서드명(파라미터)`)가 좀 더 많은 정보를 주면 좋을 것 같아요. 반환받는 값이 `int output`인데 `int`는 원시타입이라 어떤 용도인지 추측이 어렵구(객체타입이면 의도를 줄 수 있겠죠?), `output`은 무엇의 output인지 추측이 어려워...

View on GitHub

aka-nick created a review on a pull request on woowacourse-precourse/java-calculator-7
좋은 코드 잘 보고 갑니다! 👍 케이스가 잘 정리된 문서가 인상깊었어요. 간략하면서도 필요한 정보를 알기 쉬웠습니다!

View on GitHub

aka-nick created a review on a pull request on woowacourse-precourse/java-calculator-7
좋은 코드 잘 보고 갑니다! 👍 케이스가 잘 정리된 문서가 인상깊었어요. 간략하면서도 필요한 정보를 알기 쉬웠습니다!

View on GitHub

digitpic created a review comment on a pull request on woowacourse-precourse/java-calculator-7
이 부분은 저도 고민이 들었던 부분입니다.. validate 라는 메서드가 형변환한 값을 return 해버리면 검증과 형변환이라는 두가지의 책임을 지게 된다고 생각하여 두 가지 메서드로 나누어 parseInt() 메서드가 두 번 호출되는 로직이 생겼습니다 결국은 하나의 메서드가 하나의 책임을 지도록 하기 위해 이처럼 구현했습니다! 중복...

View on GitHub

digitpic created a review on a pull request on woowacourse-precourse/java-calculator-7

View on GitHub

mixxeo created a review comment on a pull request on woowacourse-precourse/java-calculator-7
작은 서비스인만큼 프로젝트를 간결하게 유지하려고 하다보니 객체에 책임이 많아졌네요 😅 우선 파서가 몰라도되는 유효성 검증 로직을 분리하면 일차적으로 개선이 될 것 같아요! 고민해보겠습니다 :)

View on GitHub

mixxeo created a review on a pull request on woowacourse-precourse/java-calculator-7

View on GitHub

songsunkook created a review comment on a pull request on woowacourse-precourse/java-calculator-7
꼼꼼한 예외처리 좋네요! 👍

View on GitHub

songsunkook created a review comment on a pull request on woowacourse-precourse/java-calculator-7
2의 의미가 모호한 것 같아요. 상수화해보는 건 어떨까요??

View on GitHub

songsunkook created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`//`나 `\\n`를 상수화하면 더 직관적인 코드를 만들 수 있을 것 같습니다!

View on GitHub

songsunkook created a review comment on a pull request on woowacourse-precourse/java-calculator-7
관심사에 대한 클래스 분리 좋네요! 👍

View on GitHub

songsunkook created a review comment on a pull request on woowacourse-precourse/java-calculator-7
기본 값을 상수로 선언해두고 그 값을 가져와 사용하는 건 어떨까요??

View on GitHub

songsunkook created a review comment on a pull request on woowacourse-precourse/java-calculator-7
List가 아니라 ArrayList를 사용한 이유가 있을까요??

View on GitHub

songsunkook created a review comment on a pull request on woowacourse-precourse/java-calculator-7
stream을 활용하면 코드를 더 간결하게 작성할 수 있을 것 같습니다! ☺️

View on GitHub

songsunkook created a review comment on a pull request on woowacourse-precourse/java-calculator-7
IllegalArgumentException을 잡아 IllegalArgumentException을 던지는 것 같은데 어떤 로직을 구상하신 건지 궁금합니다!

View on GitHub

songsunkook created a review on a pull request on woowacourse-precourse/java-calculator-7
코드 잘 읽었습니다. 예외처리를 꼼꼼하게 진행하려고 노력하신 부분이 인상깊었습니다! 👍 1주차 과제 고생하셨습니다!

View on GitHub

songsunkook created a review on a pull request on woowacourse-precourse/java-calculator-7
코드 잘 읽었습니다. 예외처리를 꼼꼼하게 진행하려고 노력하신 부분이 인상깊었습니다! 👍 1주차 과제 고생하셨습니다!

View on GitHub

kgy1008 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
생성자에 접근 제약을 설정하는 것을 놓쳤네요. 다음 미션부터 적용해보겠습니다!

View on GitHub

kgy1008 created a review on a pull request on woowacourse-precourse/java-calculator-7

View on GitHub

youngsu5582 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
구분자 생성, 숫자 생성을 담당하는 팩토리 가 있어도 나쁘지 않을거 같습니다. ( 원래는 팩토리 패턴을 안좋아한, 특수한 요구사항(커스텀 구분자)이 있기 때문에 )

View on GitHub

youngsu5582 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
해당 부분이 다소 어렵게 느껴지네요. 메소드를 분리해서 메소드 명으로 명시적으로 드러내도 괜찮을 거 같습니다.

View on GitHub

youngsu5582 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
InputView 와 OutputView 를 통해 명확하게 처리할 수 있을거 같습니다.

View on GitHub

youngsu5582 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
테스트를 위한 getter 로 보이는데 구분자와 숫자 역시도 객체로 분리한다면 필요 없어질 거 같습니다. 구문자들을 가지는 일급 객체가 있다면 이를 생성하고 이 값을 검증하면 될 거 같습니다.

View on GitHub

youngsu5582 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
해당 부분에서 문자열 -> 숫자로 변환하는 것도 역시 객체가 담당할 수 있을거 같아요. ```java public clsss PositiveNumber{ ... } ``` 여기서, 음수 변환 예외 또는 숫자 변환 오류 등을 담당할 수 있을거 같습니다.

View on GitHub

Load more