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

youngsu5582 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
이와 같은 값들은 상수로 나타내고 될 거 같습니다. index 1이 뭘 의미하는지, `/` 가 뭘 의미하는지 에 대해서 변수명으로 나타낼 수 있을겁니다. `\\` 를 CUSTOM_DELIMITER_SUFFIX 와 같이요 🫡

View on GitHub

youngsu5582 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
delimeter 도 외부에서 주입해서 생성자에서 매개변수 로 가져도 될 거 같은데 이와 같이 내부에서 초기화 하는 이유가 있나요?

View on GitHub

youngsu5582 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`StringAddCalculator` 라고 되어있지만 단순히 DTO 와 다를게 없을거 같습니다. 조금 더 역할을 가지도록 변경해도 좋을거 같아요. 👍

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 on a pull request on woowacourse-precourse/java-calculator-7
안녕하세요 6기 코레아 팀의 조이썬 입니다. 코드 잘봤습니다 🙂 중간 중간 신경 쓴 부분들이 잘 보인거 같습니다. 현재, MVC 패턴(?) 을 도입하려고 한 거 같은데 Controller - Service 가 조금 더 역할을 가져도 좋을거 같습니다! 추가로, 객체지향적인 관점을 가지려고 노력해도 괜찮을 거 같습니다. 현재는, 서비스가 모든...

View on GitHub

youngsu5582 created a review on a pull request on woowacourse-precourse/java-calculator-7
안녕하세요 6기 코레아 팀의 조이썬 입니다. 코드 잘봤습니다 🙂 중간 중간 신경 쓴 부분들이 잘 보인거 같습니다. 현재, MVC 패턴(?) 을 도입하려고 한 거 같은데 Controller - Service 가 조금 더 역할을 가져도 좋을거 같습니다! 추가로, 객체지향적인 관점을 가지려고 노력해도 괜찮을 거 같습니다. 현재는, 서비스가 모든...

View on GitHub

sjmmics created a review comment on a pull request on woowacourse-precourse/java-calculator-7
추출한다는 의미에서 메소드 이름을 extract라고 지으신 것 같네요. 컨벤션 상 get이 무난한 선택이긴 하지만 의미 전달은 이쪽이 훨씬 선명하긴 하네요.

View on GitHub

sjmmics created a review comment on a pull request on woowacourse-precourse/java-calculator-7
입력값 형식에 대한 정의와 오류 메시지를 자세하게 분류하셨네요. 세심하고 사려깊은 접근과 노력이 돋보이네요.

View on GitHub

sjmmics created a review comment on a pull request on woowacourse-precourse/java-calculator-7
똑같은 로직을 시험하는 경우 다른 입력값을 반복해서 처리해주는 @ParameterizedTest라는 기능이 있습니다. 검색해보면 반복적인 코드 작성을 피할 수 있어서 도움이 될 것 같습니다.

View on GitHub

sjmmics created a review comment on a pull request on woowacourse-precourse/java-calculator-7
mvc 패턴에서 콘트롤러가 view의 요청을 받아서 service 나 repository와 메시지를 주고 받는 것을 생각해보면 view 클래스와 의존관계를 설정하는 건 어떨까요? input view -> controller (과정 생략) -> outputview 요청과 반환이 일어나니까요.

View on GitHub

sjmmics created a review on a pull request on woowacourse-precourse/java-calculator-7
예외테스트, 입력값 검증할 때 예외 유형을 자세하게 분류하신 것이 눈에 띕니다. 컨트롤러에 너무 많은 책임이 있다는 점만 수정하시면 훨씬 좋은 코드를 짜실 수 있을 것 같습니다. 건승을 빕니다.

View on GitHub

sjmmics created a review on a pull request on woowacourse-precourse/java-calculator-7
예외테스트, 입력값 검증할 때 예외 유형을 자세하게 분류하신 것이 눈에 띕니다. 컨트롤러에 너무 많은 책임이 있다는 점만 수정하시면 훨씬 좋은 코드를 짜실 수 있을 것 같습니다. 건승을 빕니다.

View on GitHub

yushinc created a review comment on a pull request on woowacourse-precourse/java-calculator-7
답변 감사합니다. 첨부해주신 링크 참고해서 개선해보도록 하겠습니다!

View on GitHub

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

View on GitHub

eunseobb created a review comment on a pull request on woowacourse-precourse/java-calculator-7
의존성 주입으로 리팩토링하는 과정에서 놓친 부분이 많은 것 같네요,, 2주차부터는 꼼꼼하게 리팩토링 해봐야겠습니다 ! 감사합니다😀

View on GitHub

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

View on GitHub

phk1128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
그것도 좋은 방법이겠네요~ 피드백 감사해요~☺️

View on GitHub

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

View on GitHub

phk1128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
코드를 되게 꼼꼼히 읽어보셨나보네요~ 지금 보니까 COMMA, COLON이 독립적으로 쓰이는 곳이 없네요~ 민서님이 말씀하신대로 수정하면 가독성이 더 올라가겠네요 ㅎㅎ 좋은 피드백 감사합니다~

View on GitHub

daeuun created a review comment on a pull request on woowacourse-precourse/java-calculator-7
목적을 정확하게 전달하는 네이밍 작성에 유의해서 작성해야겠네요! 감사합니다

View on GitHub

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

View on GitHub

daeuun created a review comment on a pull request on woowacourse-precourse/java-calculator-7
이 부분은 다른 곳에서 커스텀 메세지를 가지는지 이미 확인하는데, 또 내부에서 검증하고 있어서 로직이 중복되는 것과도 연관되는 문제인거 같아요. 제안 감사합니다!

View on GitHub

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

View on GitHub

phk1128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
StringSumCalculator에서 double 타입으로 연산이 되고 있어서, `.`을 구분자로 사용못하게 하신것 같은데요~ 근데 미션의 입력 요구사항이 "구분자와 양의 정수" 였잖아요 ?? 그래서 일단은 요구사항에 맞게 양의 정수 연산으로 구현하는게 맞다고 생각해요~ 만약 확장을 고려하고 싶다면, 제네릭이나 메서드 오버로딩을 사용하시는게 어...

View on GitHub

phk1128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
Pattern 타입은 컴파일시에 패턴 분석이 들어가서, 비용이 큰 객체인데요~ 요고를 개선할려면 어떻게 하는게 좋을까요 ??

View on GitHub

phk1128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
StringParser에는 파싱, 유효성 검증, 데이터 변환이 혼재되어 있어서, 해당 객체의 책임이 많다고 느껴져요 SRP를 지키기 위해 어떻게 개선하면 좋을까요 ??

View on GitHub

phk1128 created a review on a pull request on woowacourse-precourse/java-calculator-7
이미 다른분들이 개선 포인트를 잘 짚어주셔서, 리뷰드릴 부분이 많이 없었어요~ 제가 말씀드린게 정답은 아니지만 민서님에게 도움이 됐으면 좋겠네요 ㅎㅎ 그럼 2주차도 화이팅하세요~

View on GitHub

phk1128 created a review on a pull request on woowacourse-precourse/java-calculator-7
이미 다른분들이 개선 포인트를 잘 짚어주셔서, 리뷰드릴 부분이 많이 없었어요~ 제가 말씀드린게 정답은 아니지만 민서님에게 도움이 됐으면 좋겠네요 ㅎㅎ 그럼 2주차도 화이팅하세요~

View on GitHub

nns503 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
메서드명에서 어떤 연산이 적용되지는 명확히 나타내면 가독성이 더 좋을 것 같습니다.

View on GitHub

Load more