Ecosyste.ms: Timeline
Browse the timeline of events for every public repo on GitHub. Data updated hourly from GH Archive.
ChoiMGyu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
네 감사합니다! 상수로 나타내는 방법은 확장이나 유지보수 측면에서 정말 좋다고 생각합니다. 하지만, 지금 프로그램에서 index 1과 같은 숫자를 상수로 나타내면 오버엔지니어링이 되지 않을까 생각합니다. 왜냐하면, 현재 index 1은 특별한 의미를 가지고 있지만 계산기를 구현하는 데 다른 곳에서 쓰일 일은 없다고 생각합니다. 오히려, ...
jintakkim created a review comment on a pull request on woowacourse-precourse/java-calculator-7
변수정할때 1,2쓰면서도 마음에 걸렸는데 다음부터 주의해야겠네요
ChoiMGyu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
일단 생성자에서 초기화한 delimeter는 기본 구분자입니다. 또한, 서비스 계층에서 입력 값의 형태에 따라 추가 구분자가 있는 경우 구분자를 추가해주었습니다. 이 과정에서 만들어진 StringAddCalculator는 올바른 입력 형태만 가지게 된다면 '기본 구분자는 무조건 가진다'라고 생각하였습니다. 그래서 위처럼 구현했습니다. 하지...
sjmmics created a review on a pull request on woowacourse-precourse/java-calculator-7
잘 봤습니다. mvc 패턴을 적용하시려고 노력하신 점이 보이네요. 네이밍 커벤션을 잘 지키고 이름을 잘 지어서 가독성 있는 코드를 작성하려고 한 것 같습니다. 원시값을 감싸는 클래스를 구현한 것도 제가 추구하는 방향과 비슷하네요. 다음 미션 코드들도 빨리 보고 싶습니다.
sjmmics created a review on a pull request on woowacourse-precourse/java-calculator-7
잘 봤습니다. mvc 패턴을 적용하시려고 노력하신 점이 보이네요. 네이밍 커벤션을 잘 지키고 이름을 잘 지어서 가독성 있는 코드를 작성하려고 한 것 같습니다. 원시값을 감싸는 클래스를 구현한 것도 제가 추구하는 방향과 비슷하네요. 다음 미션 코드들도 빨리 보고 싶습니다.
ChoiMGyu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
생각보다 더 적은 역할을 하는 메소드로 분리할 수 있었네요! 혹시 이런 메소드를 숫자를 변환하는 부분 + 덧셈을 진행하는 부분으로 나눈다라고 할 때, 커밋은 두 번 이루어지나요? 아니면 한 번만 이루어지나요?
greensnapback0229 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`extractNumbers`메서드에서 커스텀 구분자 유효성을 체크하는 책임을 갖는 것이 약간은 물음표로 남습니다. `Validation.class`의 하위 메서드를 추가해 책임 분리를 이룰 수 있지 않을까 하는 생각도 듭니다!
greensnapback0229 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`run`메서드의 이름 변경을 고려해볼 수 있다고 생각합니다. CalculatorController가 이벤트에 대해서 하나의 기능만 할 수 있는 클래스로 생각되게 하는거 같습니다. 해당 메서드에서 사용하는 domain,view 객체 조합이 어떤 동작을 나타내는지 표현할 수 있는 이름이면 좋을거 같습니다
greensnapback0229 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
//가 아닌 /로 시작하는 잘못된 입력의 경우에 올바른 예외처리가 어려워 보입니다! ex) /e\n1,2:4
ChoiMGyu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
감사합니다. 혹시 에러 메시지 같은 경우에는 위처럼 클래스에 static final 형태로 하시는 걸 선호하시는지 아니면 Enum 클래스로 정의하는 것을 선호하시는지 궁금합니다!
junghunim07 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
네! Separator는 "문자열을 분리해서 수로 반환해라" 라는 메시지만 담당하고자 설계했습니다 !
koosco created a review comment on a pull request on woowacourse-precourse/java-calculator-7
반복문 안이 로직을 별도의 메서드로 분리하면 코드 가독성을 개선시킬 수 있을 것 같아요!
koosco created a review comment on a pull request on woowacourse-precourse/java-calculator-7
if-else를 사용하는 것보다 return을 사용해 if문만을 사용해 작성하면 가독성을 높일 수 있을 것 같습니다
koosco created a review comment on a pull request on woowacourse-precourse/java-calculator-7
delimiter를 파싱하는 것과 숫자 배열을 나누는 기능을 나누어도 좋을 것 같습니다
koosco created a review comment on a pull request on woowacourse-precourse/java-calculator-7
Console.readLine을 사용해야할 것 같습니다
koosco created a review comment on a pull request on woowacourse-precourse/java-calculator-7
뒤에 \\n이 오지 않으면 잘못 파싱할 수도 있을 것 같습니다
junghunim07 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
입력값을 검증을 해야하는데 view에서 하기에는 view는 입력과 출력만 담당을 하고자 해서 Controller로 빼서 검증을 햇습니다 !
ChoiMGyu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
해당 과정을 사용자로부터 입력을 받고 validation 검사를 먼저 수행하면 되겠다고 생각했습니다. 하지만, 구현 과정에서 '문자열을 분리하고 유효성 검사를 하는 것도 서비스이지 않을까?'라고 생각해서 위와 같이 구현했습니다. 이 과정에서 최대한 중복되는 부분을 제거하고 간단하게 만들려고 했는데, 아직 부족한 부분이 있었네요. 답변해주셔서 ...
junghunim07 created a review comment on a pull request on woowacourse-precourse/java-calculator-7
의존성을 외부에서 주입해주고자 했습니다 !