Skip to content

Conversation

yslee96
Copy link

@yslee96 yslee96 commented Feb 1, 2023

기본 요구 사항 완료 후 제출 드립니다.
피드백 받고 추가 요구사항 구현에 반영하겠습니다!!

yslee96 and others added 2 commits February 1, 2023 12:27
"설명 및  체크리스트 작성"
System.out.println("결과 : " + result);
}

public static String[] receiveInput() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

프로그램 실행 시 을 입력 받는 기능을 메서드로 분리 하셨군요. 좋습니다.!

몇가지 고민해볼 만한 것들이 있는 것 같습니다.
만약에 지속적으로 재시도가 가능해야 한다면? 입력 방식에 대한 규칙이 생겨서 유효성 검사 같은 것을 해야 한다면?

이미 작성되어 있는 코드를 수정하지 않고 추가적인 요구사항에 대해서 추가 및 수정이 가능하도록
기존 클래스에서 분리하여 입력과 관련된 책임을 갖는 클래스로 분리해보시는 것은 어떠하신지요?

PLUS("+", (first, second) -> first + second),
MINUS("-", (first, second) -> first - second),
MULTIPLY("*", (first, second) -> first * second),
DIVIDE("/", (first, second) -> first / second);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DIVIDE("/", (first, second) -> first / second);
DIVIDE("/", (first, second) -> {
if(second == 0){
throw new IllegalArgumentException("0으로 나눌 수 없습니다.");
}
return first / second;
});

나누기는 예외가 발생할 수 있는 케이스가 존재하는 것 같습니다.!

private String symbol;
private BiFunction<Integer, Integer, Integer> operation;

Operator(String symbol, BiFunction<Integer, Integer, Integer> operation) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수형 인터페이스를 사용할 때 몇 가지 고민해볼 수 있는 내용이 있습니다.

  1. 표준 함수형 인터페이스를 사용한다.
  2. 표준 함수형 인터페이스에서도 제네릭의 타입이 모두 같은 경우 이를 하나로 관리할 수 있는 인터페이스를 사용한다.(BinaryOperator)
  3. primitive 타입의 wrapper 클래스인 Integer 타입은 형변환의 효율을 높일 수 있는 인터페이스가 존재한다.(IntBinaryOperator)
  4. 인터페이스의 명칭이라던가 메서드를 보다 명시적으로 관리하기 위해 @FunctionalInterface 어노테이션을 사용하여 사용자 정의 인터페이스를 작성한다.

this.operation = operation;
}

public String getSymbol(){ return symbol; }
Copy link

@SeokRae SeokRae Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public String getSymbol(){ return symbol; }
public static Operator findSymbol(String input) {
return Arrays.stream(values())
.filter(operator -> operator.symbol.equals(input))
.findAny()
.orElseThrow(() -> new IllegalArgumentException("연산자가 아닙니다."));
}

현재 Operator 에 정의된 내용을 이 Operator를 사용하는 클라이언트 클래스에서 가져다가 로직을 만드는게 아니라
내부적으로 연산자를 찾아서 반환하는 코드를 작성하면 밖에서 반복문 및 조건문을 더 줄일 수 있을 것 같네요.!


import java.util.function.BiFunction;

public enum Operator {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operator 같은 정의된 속성과 행위를 함께 관리하고 있는 클래스를 구현했다면
테스트 코드는 필히 들어가야 할 것 같습니다. 2주차 내용이 테스트 코드이니

build.gradle 파일 내에 아래 의존성 추가하신 후에 코드를 실행해보시는 것을 권장합니다.

dependencies {
    testImplementation 'org.junit.jupiter:junit-jupiter:5.8.1'
    testImplementation 'org.assertj:assertj-core:3.20.2'
}
class OperatorTest {
	
	private Stream<Arguments> operatorProvider() {
		return Stream.of(
			Arguments.of("+", Operator.PLUS),
			Arguments.of("-", Operator.MINUS),
			Arguments.of("*", Operator.MULTIPLY),
			Arguments.of("/", Operator.DIVIDE)
		);
	}
	
	@DisplayName("연산자 확인 테스트")
	@MethodSource("operatorProvider")
	@Test
	void testCase1(String actual, Operator expected) {
		Operator operator = Operator.findSymbol(actual);
		assertThat(operator).isEqualTo(Operator.PLUS);
	}
	
}

연산자를 찾는 검증 테스트를 수행했으니 연산자와 피연산자를 전달하여 연산처리하는 테스트도 작성해보실 수도 있을 것 같네요. 한번 시도해보시죠!!

Copy link

@SeokRae SeokRae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 윤수님 과제 고생하셨습니다.

Operator에 대한 커멘트를 드려봤는데 이를 기반으로 Calculator를 수정해보실 만한 것을이 있으실 것 같습니다.

  • 사용자에게 입력받은 식을 내부에서는 걱정없이 유효한 값들만 들어왔음을 보장할 수 있도록 처리하는 유효성 검사의 역할을 담당하는 객체
  • 연산자와 피연산자의 연산처리를 담당하는 객체
  • 입력 받은 값을 반복적으로 연산처리 요청을 수행하는 객체

정답이 있는 내용은 아니고 이렇게 분리해 나아가 볼 수도 있다고 제안 드리는 것이니 한번 고민해보시기바랍니다.!

public int calculate(String[] inputExp){

int result = 0;
// 맨처음 숫자 더해짐
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

공부의 목적으로 주석이 좋지만, 코드를 이용해 의도를 나타내는게 중요한듯 합니다.


코드로 의도를 표현하라!
// 직원에게 복지 혜택을 받을 자격이 있는지 검사한다.
if((employee.flags & HOURLY_FLAG) && (employee.age > 65))
주석도 필요없이 함수 이름만으로 충분히 깔끔하게 표현되었다.
if (employee.isEligibleForFullBenefits())

이런식으로 말이죠 ㅎㅎ


public class Calculator {

private static final Pattern regExp = Pattern.compile("^[0-9]*$"); //operand, operator 구분 용도
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +21 to +24
for(Operator operator : Operator.values()){
if(operator.getSymbol().equals(input)){
currentOperator = operator;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

연산자 매칭을 values를 stream을 돌면서 findFirst 메서드를 이용해 해보면 어떨까요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants