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

cherryiJuice created a review comment on a pull request on woowacourse-precourse/java-calculator-7
> `for`문이 아닌 `stream`을 사용하셨네요! > > 두 로직의 차이점에 대해 알고 계신가요? 👀 각각 어떤 장단점을 가지고 있을까요? > > 추가로, 저는 위 코멘트와 반대 의견을 가지고 있는데요. (누가 정답이라고 주장하는 건 아닙니다! 코딩은 그런 거니까요. ☺️) > > 메서드명이나 변수명에 타입을 지정하는 방식은 권장...

View on GitHub

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

View on GitHub

cherryiJuice created a review comment on a pull request on woowacourse-precourse/java-calculator-7
오 그런 방식이 있을 수 있겠군요!! 감사합니다 ```java public int sum() { return numbers.stream().mapToInt(Integer::intValue).sum(); } ``` 일급클래스일 때 `Numbers` 클래스에서는 이런 식으로 해당 연산을 처리하고 ```java public c...

View on GitHub

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

View on GitHub

yeonnhuu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
클래스를 확장하는 경우를 고려한다면 유효성을 검증하는 별도의 클래스로 분리하는 게 맞는 것 같습니다. 의견 감사합니다! 😊

View on GitHub

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

View on GitHub

na0th created a review comment on a pull request on woowacourse-precourse/java-calculator-7
저도 이 부분 좋다고 생각합니다~ 메세지를 따로 빼지않고 가까운 곳에서 관리하는 게 좋았습니다. 코드 보는 입장에서 왔다갔다 안해도 되는 게 좋았습니다

View on GitHub

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

View on GitHub

kwongio created a review on a pull request on woowacourse-precourse/java-calculator-7
안녕하세요 수빈님 코드 잘 봤습니다! 1주차 미션 고생많으셨어요 2주차 미션도 화이팅입니다!

View on GitHub

minSsan created a review comment on a pull request on woowacourse-precourse/java-calculator-7
숫자 `2`는 `constant`에 정의하신 값으로 충분히 **상수화** 할 수 있을 것 같습니다. - 구분자 관련 값의 관리 지점을 한 곳으로 모으는 게 좋을 것 같아요.

View on GitHub

minSsan created a review comment on a pull request on woowacourse-precourse/java-calculator-7
테스트 코드도 클래스별로 나눠서 작성해보시는 건 어떨까요? 모든 객체가 통합된 `Application`의 run 메소드만 테스트하게 되면, 테스트가 통과하지 못할 경우 **정확히 어느 객체에서 오류가 발생**하는지 확인하기 어렵다고 생각합니다.

View on GitHub

minSsan created a review comment on a pull request on woowacourse-precourse/java-calculator-7
계층형 구조를 그대로 가져가신다면, - `CalculatorService` 클래스에서는 **비즈니스 로직을 수행한 결과 값을 리턴**하도록 하고, - `CalculatorController` 내부에서 뷰어(ex. `CalculatorInput` / `CalculatorOuput`)와 `CalculatorService` 객체를 참조하여 - 요구사...

View on GitHub

minSsan created a review comment on a pull request on woowacourse-precourse/java-calculator-7
`CalculatorService` 객체에서 뷰어의 역할도 담당하고 있는 것 같습니다. `CalculatorService` 클래스에는 **비즈니스 로직만** 남겨두고, 입력 / 출력을 담당하는 뷰어는 따로 분리하는게 어떨까요?

View on GitHub

minSsan created a review on a pull request on woowacourse-precourse/java-calculator-7
안녕하세요, 먼저 리뷰 남겨주셔서 정말 감사합니다 🙇🏻‍♀️ 앞서 다른 분께서도 리뷰를 많이 남겨주셔서, 겹치는 부분은 최대한 제외하고 리뷰 남겨드렸습니다. 부족하지만 도움이 되셨으면 좋겠습니다. 2차 과제도 파이팅입니다!

View on GitHub

minSsan created a review on a pull request on woowacourse-precourse/java-calculator-7
안녕하세요, 먼저 리뷰 남겨주셔서 정말 감사합니다 🙇🏻‍♀️ 앞서 다른 분께서도 리뷰를 많이 남겨주셔서, 겹치는 부분은 최대한 제외하고 리뷰 남겨드렸습니다. 부족하지만 도움이 되셨으면 좋겠습니다. 2차 과제도 파이팅입니다!

View on GitHub

kwongio created a review comment on a pull request on woowacourse-precourse/java-calculator-7
addBasicSeparator을 여러번 호출하면 separator에 중복된 값이 계속 들어갈 것 같습니다 SeparatorParser의 기본생성자로 값을 세팅하거나 변하지 않는 값이라면 separator에 직접 값을 넣는 게 좋을 것 같습니다

View on GitHub

kwongio created a review comment on a pull request on woowacourse-precourse/java-calculator-7
@DisplayName을 사용하면 @DisplayName("여러개 커스텀 구분자 사용") 이렇게 더 편하게 테스트 이름을 지을 수 있습니다

View on GitHub

kwongio created a review comment on a pull request on woowacourse-precourse/java-calculator-7
Stack의 내부에는 synchoronized로 되어있어서 단일 쓰레드에서는 잘 사용하지 않습니다 ArrayDeque와 같은 자료구조를 사용해보시는건 어떠신가요?

View on GitHub

kwongio created a review comment on a pull request on woowacourse-precourse/java-calculator-7
for문에 i, j, k 이렇게 관례로 쓰긴하지만 로직을 작성할 때는 의미있는 이름을 주는 것도 고려해보면 좋을 것 같습니다!

View on GitHub

kwongio created a review comment on a pull request on woowacourse-precourse/java-calculator-7
removedString은 이미 String형이니까 removed로 해도 좋을 것 같습니다

View on GitHub

kwongio created a review comment on a pull request on woowacourse-precourse/java-calculator-7
팩토리 메소드 패턴을 사용해서 CalculatorController를 더 쉽게 생성할 수 있을 것 같습니다!

View on GitHub

kwongio created a review comment on a pull request on woowacourse-precourse/java-calculator-7
메소드 prefix에 is는 주로 boolean형에 많이 쓰여서 메소드 호출할 때 헷갈릴 것 같아요 그리고 tmpString보단 좀 더 의미있는 이름을 지어주면 읽기 쉬운 쉬코드가 될 것 같습니다

View on GitHub

kwongio created a review on a pull request on woowacourse-precourse/java-calculator-7
안녕하세요 수비님 코드 잘 봤습니다! 1주차 미션 고생많으셨어요 2주차 미션도 화이팅입니다!

View on GitHub

kwongio created a review on a pull request on woowacourse-precourse/java-calculator-7
안녕하세요 수비님 코드 잘 봤습니다! 1주차 미션 고생많으셨어요 2주차 미션도 화이팅입니다!

View on GitHub

na0th created a review comment on a pull request on woowacourse-precourse/java-calculator-7
상세하게 readme 적어주셔서 코드 파악하는데 더욱 수월한 것 같습니다. 보고 배우겠습니다^^

View on GitHub

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

View on GitHub

yeonnhuu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
extractDelimiters와 extractNumbers 메서드 모두 공통적으로 input.startsWith("//") 확인 조건이 있어서 별도의 메서드로 분리했습니다!

View on GitHub

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

View on GitHub

yeonnhuu created a review comment on a pull request on woowacourse-precourse/java-calculator-7
과제명 자체가 문자열 계산기여서 StringCalculator라고 작성했는데 다음 번에 클래스 작성할 때 메서드나 클래스명에 예약어나 기본 제공 클래스명을 사용하는 것을 주의하도록 해야겠네요. 의견 감사합니다!!

View on GitHub

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

View on GitHub

Load more