Skip to content

feat :: 레이싱카 기능구현#1

Open
ca1af wants to merge 1 commit intoCODE-CLEANERS:mainfrom
minseon33:tryOne
Open

feat :: 레이싱카 기능구현#1
ca1af wants to merge 1 commit intoCODE-CLEANERS:mainfrom
minseon33:tryOne

Conversation

@ca1af
Copy link
Collaborator

@ca1af ca1af commented Oct 8, 2023

No description provided.

Copy link
Collaborator Author

@ca1af ca1af left a comment

Choose a reason for hiding this comment

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

전체적으로 노력하신 느낌을 받아서 좋았습니다. 기능 구현을 완성하신 부분 너무 멋집니다

더해서, 원시값을 포장해서 Enum 으로 빼는 습관 역시 멋집니다.

한 번 같이 고민해보고 싶은 것은

객체지향적 설계란 무엇일까요?
객체 지향은 왜 해야할까요?
스프링에서 주로 사용해온 레이어드 아키텍쳐에서 service 레이어에 모든 로직을 넣게되면 어떤 단점이 있을까요?

위 의문들에 대해서 좀 더 고민해보면서, 디테일한 부분들(ex: 네이밍)도 신경쓰면 어떨까 싶습니다


public class ValidationUtils {

public static void lengthValid(String carName) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

자바에서 매서드를 네이밍하는 규칙은 뭘까요?

validateLength, lengthValidation 정도가 영어 문법적으로는 맞아 보이는데, 네이밍 규칙에 따르면 어느 쪽이 더 나은 네이밍일지 고민해보면 어떨까요?

import common.ValidationUtils;

public class Car {
String name;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

접근제어자 (private, protected, public...) 를 사용하면 어떨까요?

final 을 사용할 수 있는 곳에는 final을 붙이면 어떨까요?


public Car(String name, int forwardCondition) {
ValidationUtils.lengthValid(name);
ValidationUtils.validateNotBlank(name);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 두 매서드는 하나로 빼는것이 어떨까요?

예를 들어 Utils.validate(name) 으로 뺄 수 있지 않을까요?

해당 유틸 클래스 안에서는 두 개의 매서드로 나누되, 사용하는 측에서는 validate() 만 호출하면 더 편하지 않을까요?


public class Car {
String name;
int forwardCondition;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

forwardCondition 이 무엇을 의미하는지 조금 모호한 네이밍 같습니다.

다른 더 좋은 네이밍을 사용하는 건 어떨까요?

import java.util.List;

public class Cars {
private final List<Car> cars;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일급 컬렉션을 만들었지만 사용하지 않는 것 같습니다.

이 일급 컬렉션은 무슨 일들을 할 수 있을까요?

일급 컬렉션에 대해 좀 더 자세히 알아보고 실습해보면 어떨까요?

public class InputView {

private BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
private BufferedWriter wr = new BufferedWriter(new OutputStreamWriter(System.out));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

필요 없는 객체는 정리하는 것이 어떨까요?

//인풋 이넘을 활용해서 "경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분)." 이거 뜨게 한다.
try{
// wr.write(InputMassage.START_GAME.getInputMassage());
System.out.println(InputMassage.START_GAME.getInputMassage());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

InputView 에서는 사용자에게 뭔가를 출력하지 않는 것이 어떨까요?

List<Car> carList = carService.createCar(inputName);

//then
org.assertj.core.api.Assertions.assertThat(carList).hasSize(4);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alt + Enter 로 static import 로 추가하면 assertThat() 으로만 쓸 수 있습니다.

Assertions.assertThatThrownBy(()-> carService.createCar(inputName))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(ErrorMassage.LENGTH_VALID_ERROR.getErrorMessage());
System.out.println(ErrorMassage.LENGTH_VALID_ERROR.getErrorMessage());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

System.out 할 필요가 있을지 고민해보면 어떨까요?

import java.util.Arrays;
import java.util.List;

class CarServiceTest {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

자동차의 이름에 대한 로직은 존재하지만 핵심 로직 (이동) 에 대한 로직 테스트는 존재하지 않는 듯 합니다.

어떻게 하면 핵심 로직을 테스트 할 수 있을까요?

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.

2 participants