Ecosyste.ms: Timeline
Browse the timeline of events for every public repo on GitHub. Data updated hourly from GH Archive.
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
문제될 부분은 아니지만 개인적으로 add는 더하는 역할만 있어야할텐데, 현재 Number객체를 추가하고 큰 수를 더하는 역할을 진행하고 있는데 SoC를 도입할 수 있을 것이라는 생각이 듭니다.
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
윗 코멘트에 동의합니다!!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
커스텀 문자는 UI측에서 구분한다. 굉장히 좋은 접근인 것 같습니다. 저는 모두 다 비즈니스 로직으로 처리하면서 도메인이 무거워졌던 것 같은데요. UI와 Buisness의 경계를 되게 잘 고려하신 것 같아요. 현재 진행중인 2주 차 과제에서 참고해보겠습니다!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
이 부분에 대해 저도 코멘트를 남깁니다! 아마도 MVC v1~v2 정도의 설계를 의도하신 것 같은데 맞으실까요?! 하지만, 제가 아는 MVC 변화에 따르면 조금 더 괜찮은 구조로 개선할 수 있다고 봅니다! 참고가 되었으면 좋겠습니다. :) 트렌드에 맞는 기술력을 적용하신 부분 좋은 것 같습니다!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
> 일급 객체 사용 좋은 것 같습니다. 저는 중간에 요구사항 변경으로 int를 long로 수정함에 있어서 사용된 모든 함수를 찾으러 다녔었거든요.😂😂 이번 주차부터는 잘 활용해야겠습니다. 저도 같은 문제점으로 고생했었는데 ㅠㅠ 좋은 코드네요 👍
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
저는 위에 작성하신 유정님과 조금 다른 의견을 가지고 있습니다! 제가 바라보는 포인트는 아래와 같습니다. 1. 현재 Service에서 의존성을 필요로 할 때, DIP를 지킬 수 있는가? -> 현재와 같은 구조면 문제 없다고 봅니다. 2. 그럼 의존성 주입을 위한 클래스는 왜 필요한가? -> 저는 대개, 확장성을 고려하고 도입한다고 생각합니...
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
UI까지 테스트!!! 대박입니다. 그런데 단위 테스트, 통합 테스트, 인수 테스트의 장,단점을 활용해보시면 좋을 것 같아요!!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
assertThatThrownBy에서 어떤 예외가 발생하는지 명시해주면 좋을 것 같습니다. 현재 코드만 봤을 땐, 비즈니스 로직에서 예상하는 예외인지 프로그램 자체에서 예상되는 예외인지 식별하기 힘든 부분이 있습니다!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
AssertJ의 메소드는 static이니까 조금 더 코드의 줄을 줄일 수 있지 않을까요?
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
정확히 어떤 것을 추출하는지 잘 모르겠습니다. 기능에 대해서 명확히 작성해주면 테스트를 이해하기 편할 것 같습니다. BDD 패턴을 추천드립니다!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
해당 코드 구조를 보면, 하나의 커스텀 구분자만 입력받도록 한 것 같습니다. 그럼 message.lastIndexOf보다 message.charAt()을 활용하면 조금 더 효율적이고 직관적인 코드가 될 것 같습니다.
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
field가 모두 상수이며, gette의 기능만 필요하다면 record를 사용해도 괜찮을 것 같습니다!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
접근 제어가 없는 부분들도 명시해주면 좋을 것 같습니다!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
static으로 선언하신 이유가 있으실까요? auto method extract를 통해 생성하신 것이라면, static이 사용되는 것이 적절한지 한 번 생각해봐도 좋을 것 같습니다!
jihwankim128 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
이펙티브 자바 3판, 아이템 11 equals를 재정의하려거든 hashCode도 재정의하라. 이미 충분히 좋은 코드지만 한 번, 참고하시면 조금 더 대중적으로 좋은 코드가 될 것 같습니다!
jihwankim128 created a review on a pull request on woowacourse-precourse/java-calculator-7
전체적으로 고급진 내용이네요. 설계에 대해 스스로 많은 피드백을 거친 흔적이 보입니다. 다만 늦게 리뷰에 참여하는 만큼 개인적으로 아쉬운 부분이 있습니다. 몇 몇 분께서 인터페이스 도입에 대해 의문을 가지고 계십니다. 저도 해당 부분에 대해서 의문점이 들게 되었는데요.. 개인적으로 만약, 테크 리더나 경험이 많은 개발자가 도입한게 아니...
jihwankim128 created a review on a pull request on woowacourse-precourse/java-calculator-7
전체적으로 고급진 내용이네요. 설계에 대해 스스로 많은 피드백을 거친 흔적이 보입니다. 다만 늦게 리뷰에 참여하는 만큼 개인적으로 아쉬운 부분이 있습니다. 몇 몇 분께서 인터페이스 도입에 대해 의문을 가지고 계십니다. 저도 해당 부분에 대해서 의문점이 들게 되었는데요.. 개인적으로 만약, 테크 리더나 경험이 많은 개발자가 도입한게 아니...
cherryiJuice created a review comment on a pull request on woowacourse-precourse/java-calculator-7
아하,, 정적 팩토리 메소드에 대한 장점들만 알고 있었어서 쓰는게 좋을 거라 생각했는데 그런 관점이 존재할 수 있겠군요 ㅎㅎ 좋은 의견 나눠주셔서 정말 감사합니다! (가능하시다면 다른 답글도 답장해주시면 정말정말 감사하겠습니다 ㅎㅎ...)
sunwon12 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
customSeparatorHandler로 custom separator를 얻고 그것을 Separator 객체에 추가하는 로직입니다. 메소드명 때문에 혼돈될 수 도 있겠네요 그리고 정환님 덕분에 Separator가 VO인지 엔티티인지 일급컬렉션인지 생각해볼 수 있었어요. 이번에는 구분하지 않고 객체를 만들었지만, 2주차 과제에서는 설계전 한...
sunwon12 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
정수와 소수를 한 변수로 커버하기 위함입니다!
sunwon12 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
정수는 정수형식으로 출력하게 하고 소수는 소수형식으로 출력하게 하기 위함입니다 ex) 6.00 -> 6으로 출력 6.12 -> 6.12으로 출력
soeunnPark created a review comment on a pull request on woowacourse-precourse/java-calculator-7
프로그램 동작을 한 눈에 볼 수 있어서 너무 좋은 것 같아요! 👍
soeunnPark created a review comment on a pull request on woowacourse-precourse/java-calculator-7
문자열 처리에 굉장히 능숙하신 것 같아요 👍
soeunnPark created a review comment on a pull request on woowacourse-precourse/java-calculator-7
정적 팩토리 메서드를 통해 객체를 생성하는 목적을 명확하게 드러낼 수 있다고 생각합니다! 기본 구분자를 만드는 것과, 커스텀 구분자를 포함해 만드는 것이 각각 객체를 생성하는 목적과 의도가 다르기 때문에 이를 정적 팩토리 메서드를 통해 분리하는 것이 좋다고 생각합니다. 다만, 말씀해주신대로 커스텀 구분자가 기본 구분자를 포함하기 때문에 현재 로직...
soeunnPark created a review comment on a pull request on woowacourse-precourse/java-calculator-7
""와 1도 상수처리하면 더 완벽해질 것 같아요.. !!