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

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

View on GitHub

jintakkim created a review comment on a pull request on woowacourse-precourse/java-calculator-7
네 그쵸 제가봐도 과한감이 있네요 ㅎㅎ

View on GitHub

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

View on GitHub

jintakkim created a review comment on a pull request on woowacourse-precourse/java-calculator-7
> 단순 setter는 save보다는 set이 일반적인 표기입니다. 단순히 값을 저장 하는 setter보다 좀더 여러기능이 있어서 save라고 정했습니다

View on GitHub

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

View on GitHub

SeoMoonk created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`final`은 불변한다는 의미를 가지고 있는데 저 친구는 구분자를 추출할 때마다 변경되는 친구기에 안붙이려고 했던 것 같습니다! 또한, `ArrayDeque`를 사용하는 이유는 `addLast()` 와 `removeFirst()` 메서드를 통해 큐 구조를 사용함으로써 `특수 구분자가 있다면, 일반 구분자보다 먼저 처리를 진행한다` 는 저만의 생각...

View on GitHub

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

View on GitHub

jintakkim created a review comment on a pull request on woowacourse-precourse/java-calculator-7
> 단순 setter는 save보다는 set이 일반적인 표기입니다. 단순히 값을 저장 하는 setter보다 좀더 여러기능이 있어서 save라고 정했습니다

View on GitHub

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

View on GitHub

jintakkim created a review comment on a pull request on woowacourse-precourse/java-calculator-7
단순히 값을 저장 하는 setter보다 좀더 여러기능이 있어서 save라고 정했습니다

View on GitHub

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

View on GitHub

SeoMoonk created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`팩토리 메소드 패턴` !! 들어본 적은 있는데 아직 정확히 배워본적은 없어서요. 한번 배워보겠습니다! 감사합니다 :)

View on GitHub

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

View on GitHub

jintakkim created a review comment on a pull request on woowacourse-precourse/java-calculator-7
> 그럼 예를 들어 하나의 입력에 대해 덧셈, 뺄셈, 곱셈 등을 한번에 제공하는 것을 염두에 두고 만드신건가요?? 이번 미션이 덧셈뿐이라 그럴일은 없겠지만 쉽게 확장할 수 있게 만들었습니다.

View on GitHub

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

View on GitHub

jintakkim created a review comment on a pull request on woowacourse-precourse/java-calculator-7
> getNumbers에서 새로운 인스턴스를 생성하신 이유가 있을까요?? 기존 NumberRepository의 저장소를 밖에서 임의로 변경하는 것을 막기위해 값을 한번 복사해서 리턴했습니다.

View on GitHub

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

View on GitHub

ChoiMGyu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
네 감사합니다! 상수로 나타내는 방법은 확장이나 유지보수 측면에서 정말 좋다고 생각합니다. 하지만, 지금 프로그램에서 index 1과 같은 숫자를 상수로 나타내면 오버엔지니어링이 되지 않을까 생각합니다. 왜냐하면, 현재 index 1은 특별한 의미를 가지고 있지만 계산기를 구현하는 데 다른 곳에서 쓰일 일은 없다고 생각합니다. 오히려, ...

View on GitHub

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

View on GitHub

jintakkim created a review comment on a pull request on woowacourse-precourse/java-calculator-7
변수정할때 1,2쓰면서도 마음에 걸렸는데 다음부터 주의해야겠네요

View on GitHub

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

View on GitHub

ChoiMGyu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
일단 생성자에서 초기화한 delimeter는 기본 구분자입니다. 또한, 서비스 계층에서 입력 값의 형태에 따라 추가 구분자가 있는 경우 구분자를 추가해주었습니다. 이 과정에서 만들어진 StringAddCalculator는 올바른 입력 형태만 가지게 된다면 '기본 구분자는 무조건 가진다'라고 생각하였습니다. 그래서 위처럼 구현했습니다. 하지...

View on GitHub

ChoiMGyu 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
잘 봤습니다. mvc 패턴을 적용하시려고 노력하신 점이 보이네요. 네이밍 커벤션을 잘 지키고 이름을 잘 지어서 가독성 있는 코드를 작성하려고 한 것 같습니다. 원시값을 감싸는 클래스를 구현한 것도 제가 추구하는 방향과 비슷하네요. 다음 미션 코드들도 빨리 보고 싶습니다.

View on GitHub

sjmmics created a review on a pull request on woowacourse-precourse/java-calculator-7
잘 봤습니다. mvc 패턴을 적용하시려고 노력하신 점이 보이네요. 네이밍 커벤션을 잘 지키고 이름을 잘 지어서 가독성 있는 코드를 작성하려고 한 것 같습니다. 원시값을 감싸는 클래스를 구현한 것도 제가 추구하는 방향과 비슷하네요. 다음 미션 코드들도 빨리 보고 싶습니다.

View on GitHub

ChoiMGyu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
생각보다 더 적은 역할을 하는 메소드로 분리할 수 있었네요! 혹시 이런 메소드를 숫자를 변환하는 부분 + 덧셈을 진행하는 부분으로 나눈다라고 할 때, 커밋은 두 번 이루어지나요? 아니면 한 번만 이루어지나요?

View on GitHub

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

View on GitHub

greensnapback0229 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`extractNumbers`메서드에서 커스텀 구분자 유효성을 체크하는 책임을 갖는 것이 약간은 물음표로 남습니다. `Validation.class`의 하위 메서드를 추가해 책임 분리를 이룰 수 있지 않을까 하는 생각도 듭니다!

View on GitHub

greensnapback0229 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`run`메서드의 이름 변경을 고려해볼 수 있다고 생각합니다. CalculatorController가 이벤트에 대해서 하나의 기능만 할 수 있는 클래스로 생각되게 하는거 같습니다. 해당 메서드에서 사용하는 domain,view 객체 조합이 어떤 동작을 나타내는지 표현할 수 있는 이름이면 좋을거 같습니다

View on GitHub

greensnapback0229 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
//가 아닌 /로 시작하는 잘못된 입력의 경우에 올바른 예외처리가 어려워 보입니다! ex) /e\n1,2:4

View on GitHub

Load more