[20230424] 우아한테크코스 5기 LEVEL 2 - 웹 자동차 경주 1단계 & 2단계 회고

안녕하세요 ~ 헤나입니다 ! 😄

 

이번에는 "LEVEL2 - 웹 자동차 경주 미션 1단계" 회고를 진행해보려고 합니다 !

 

 

 

 

 

 

🟧 01. 테스트 검증부 하드코딩

기존에 테스트를 다음과 같이 작성했다.

@DisplayName("자동차 경주를 통해 게임 결과를 반환한다.")
@Test
void race() {
    // given
    final String jeomoon = "저문";
    final String hyena = "헤나";
    final CarGroup carGroup = new CarGroup("저문,헤나");
    final int trial = 10;

    // when
    final RacingInfoResponse response = service.race(carGroup, trial);

    final List<String> names = response.getRacingCars()
            .stream()
            .map(CarInfoDto::getName)
            .collect(Collectors.toList());

    // then
    assertThat(names).contains(jeomoon, hyena);
}

 

검증부 assert 부분에 given에서 선언한 변수를 가져다가 쓰고 있다.

 

이 부분에 대해서 리뷰어는 두 가지 말씀을 해주셨다.

• given은 시나리오 진행을 위한 값을 설정하는 부분이다.
• 검증부는 하드코딩하는 편이 좋다.

💁‍♂️ BDD 에서 given 은 시나리오 진행을 위한 값을 설정하는 부분이라고 생각해요!
저는 해당 값이(성함이.. 🙇 ) 검증을 위한 값이라고 생각해요.
또 검증 부분에서 given 을 한 번 더 확인해야하는 수고로움도 발생할 수 있어요
아래와 같이 하드코딩 해주면 더 명확하지 않을까요? 😄

// then
assertThat(names).contains("저문", "헤나");

💁‍♂️ 검증을 위한 값은 특별한 상황이 아니면 하드코딩 해주는 것이 좋아요!
아래 글을 참고해보면 좋겠어요 😄

검증부 (assert / expect)는 하드코딩한다

 

검증부에 소프트 코딩을 하면 문제가 발생할 수 있다.

 

🟢 01-01. 소프트 코딩이란 ?

• 소프트 코딩은 간단히 말하자면 "도메인 로직을 사용한 방식"을 의미한다.

소프트 코딩을 정의를 한 블로그에서 가져왔다.

 

소프트 코딩은 전 처리기 매크로, 외부 상수, 데이터베이스, 명령 줄 인수 및 사용자 입력과 같은 외부 소스에서 값을 가져 오는 프로그래밍 방식입니다. 이 용어는 "하드 코딩"의 반대이거나 소스 코드에 직접 값을 넣는 것으로 사용자가 변경할 수 없습니다. 소프트 코딩이 더 유연한 것으로 간주됩니다.

 

 

🟢 01-02. 소프트 코딩의 단점

• 검증이 무의미하다.

테스트에 도메인 로직을 사용한다면 "프로덕션 코드를 복사하고 붙여넣은 것"과 다름없다.

 

문제가 없어보일 수 있다.

 

하지만 프로덕션 코드가 변경된다면 테스트 로직도 같이 변경되어야 한다.

유지보수 비용이 클뿐만 아니라 결국 같은 로직이 같은 값을 반환하는지 검증하고 있으니 무의하다.

 

 

• 잘못된 테스트가 성공한다.

버그가 있는 기능을 테스트한다고 해보자.

테스트 코드에 해당 버그가 있는 기능의 코드를 복사해서 사용한다면

잘못된 값끼리 비교하며 테스트가 성공할 수 있다.

 

이럴 경우 실제로는 오류를 찾기 어렵고 큰 장애가 생길 수 있다.

 

 

 

• Test First가 어렵다.

TDD 같은 경우 테스트를 먼저 작성한다.

 

소프트 코딩을 할 경우 위에서와 마찬가지로 프로덕션 코드가 테스트 내부에 있을 수 있다.

 

즉, 테스트를 작성하면서 미리 구현 코드를 구상하게 된다.

 

TDD는 구현부를 생각하고 작성하는 것이 아닌 예상 결과를 미리 작성하는 방법이다.

하지만 소프트 코딩을 하는 경우 테스트에 미리 구현부를 작성할 가능성이 높으므로 TDD의 의미가 사라진다.

 

 

 

 

🟢  01-03. 검증부는 하드 코딩하는 편이 좋다.

테스트 검증부를 하드 코딩한다면

• 미리 구현부를 생각하지 않을 수 있다.
• 무의미한 검증을 하지 않을 수 있다.
• 잘못된 테스트를 만들 가능성이 적다.

그러므로 단위 테스트에서는 검증부를 하드코딩 하도록 하자 !

 

 

 

 

---

 

 

 

 

 

🟧 02. Service 끼리 의존하고 있는 것에 대해서

💁‍♂️ 미리 말씀드리지 못했는데 저는 개인적인 취향의 영역이라고는 생각합니다 :)
Service 에서 도메인 객체를 인자로 받아 사용한다면 다른 Service가 해당 Service 를 의존해야할 때 사용하기 편리할 거에요
이럴 때 아래와 같은것들을 고민해보면 좋겠어요

- Service 가 같은 계층(Layer)에 있는 Service 를 의존해도 될까?
- Service 끼리 계층이 명확하다면 서로 의존해도 되는것 아닐까?
- Service 끼리 계층 표현을 어떻게 하지?계층 표현이 불가능하면 어떻게 하지?

헤나만의 정답을 찾아봤으면 좋겠어요 :)
크루들과 토론해보기도 좋은 주제라 생각합니다

 

 

 

🟢 02-01. Service가 Service를 의존할 경우 순환 참조 문제가 발생할 수 있다.

빈을 등록 시키기 위해서 
ServiceA는 ServiceB를 받으려하고
ServiceB는 ServiceA를 받으려하는 문제로
서로 계속 호출하는 경우이다.

아래와 같이 컴파일 시점에서 순환 참조에 대해서 에러를 발생시킨다.

┌─────┐
|  serviceA defined in file []
↑     ↓
|  serviceB defined in file []
└─────┘

Relying upon circular references is discouraged and they are prohibited by default. 
Update your application to remove the dependency cycle between beans. 
As a last resort, it may be possible to break the cycle automatically by setting spring.main.allow-circular-references to true.

public class ServiceA {
    private final ServiceB serviceB;
}

public class ServiceB {
    private final ServiceA serviceA;
}

 

 

🟢 02-02. Service 끼리 계층이 명확할 경우 의존해도 괜찮을까 ?

나는 Service 계층이 명확하다는 뜻이 Service와 Repository가 1:1인 경우라고 생각했다.

그리고 다음과 같이 Service가 Service를 서로 의존하는 상황을 가정해봤다.

@Service
public class UserService {
    private final PostService postService;
    private final UserRepository userRepository;

    public UserService(PostService postService, UserRepository userRepository) {
        this.postService = postService;
        this.userRepository = userRepository;
    }

    // PostService를 이용해서 사용자의 게시글을 전체 조회한다.
    public List<Post> findAllPostByUserId(long userId) {
        return postService.findAllByUserId(userId);
    }

    // PostService를 이용해서 사용자 정보와 함께 게시글을 저장한다.
    public long savePost(String title, String content, long userId) {
        User findUser = userRepository.findByUserId(userId);
        Post newPost = new Post(title, content, findUser.findUsername(), findUser.getId());
        return postService.save(newPost);
    }
}

• PostService를 이용해서 사용자의 게시글을 전체 조회한다.
• PostService를 이용해서 사용자 정보와 함께 게시글을 저장한다.

이런 식으로 PostService에 구현된 메서드를 재사용해서 중복되는 로직을 제거하는 경우 괜찮을 수도 있다 생각했다.

• UserService가 의존하는 PostService의 조회 기능만을 사용할 경우
• 순환 참조 문제가 일어나지 않는 경우

하지만 여러 단점이 보여서 개인적으로 선호하지는 않는다.

• UserService가 다른 Service 의존도가 높아진다.
• 순환 참조가 발생할 수 있다.
• UserService를 이용하기 위해서는 PostService 내부 로직을 알아야한다.
• 트랙잭션 전파가 일어날 수 있어 복잡해질 수 있다.

🟢 02-03. 계층 표현이 불가능하다면

계층 표현이 불가능한 Service 끼리 의존하게 된다면 복잡해질 가능성이 크다.

계층 표현이 가능하면 현재 Service가 의존하는 Service가 어떤 영향을 미칠지 알기 쉽다.
하지만 계층 표현이 불가능한 Service라면 어떤 영향을 미칠지 알기 힘들다.

 

 

 

 

 

---

 

 

 

 

🟧 03. Database에서 데이터 받아올 때의 객체

💁‍♂️ 해당 객체는 Dto 라는 객체에 가까워 보여요!
헤나는 어떻게 생각하시나요?

 

리뷰어님께서 말씀해주신 부분은 PlayerMapper 부분이었다.

public class PlayerMapper {
    private final int id;
    private final Name name;
    private final Position position;
    private final int racingGameId;

    // ...
}

 

여기서 PlayerMapper는 JdbcTemplate을 이용해서 테이블을 조회하여 플레이어 정보를 가져와서 객체를 만들기 위함이었다.

 

🟢 03-01. PlayerMapper를 만든 이유

PlayerMapper를 Player의 Column을 DB에서 가져올 때의 객체를 표현하는 의미라고 생각했다.
때문에 `PLAYER` 정보를 DB에서 받아올 때를 위한 객체로 만들었다.

 

CREATE TABLE IF NOT EXISTS PLAYER
(
    id             INT PRIMARY KEY NOT NULL AUTO_INCREMENT,
    name           VARCHAR(10)     NOT NULL,
    position       INT             NOT NULL,
    racing_game_id INT             NOT NULL,
    FOREIGN KEY (racing_game_id) REFERENCES RACING_GAME (id)
);

public class PlayerMapper {
    private final int id;
    private final Name name;
    private final Position position;
    private final int racingGameId;

    // ...
}

🟢 03-02. PlayerMapper의 문제점

문제점이 정확히 무엇인지 잘 생각되지 않아서 리뷰어님한테 여쭤봤다.

 

😄 PlayerJdbcRepository에서 findByRacingGameId() 메서드의 반환 타입이 List<PlayerMapper> 이기 때문인지 궁금합니다.

 

이 때 PlayerJdbcRepository 코드는 다음과 같다.

@Repository
public class PlayerJdbcRepository implements PlayerRepository {
    @Override
    // PlayerMapper를 반환한다.
    public List<PlayerMapper> findByRacingGameId(final int racingGameId) {
        final String sql = "SELECT * FROM PLAYER WHERE racing_game_id = ?";
        return jdbcTemplate.query(sql, playerRowMapper, racingGameId);
    }
}

내가 만든 `PlayerMapper`는 데이터를 받아오기 위한 객체였다.

생각을 짜냈을 때 두 가지 추론을 해봤다.

 

01. 다른 계층까지 넘어가면서 필요없는 모든 데이터를 제공할 이유는 없다.

PlayerMapper의 모든 필드가 final로 선언된 부분도 문제가 될 수 있다.

02. 다음과 같이 몇 가지 데이터만 가져오는 쿼리라면 모든 데이터를 받지 않기 때문이다.

SELECT name, position FROM PLAYER WHERE racing_game_id = ?

 

 

 

🟢 03-03 PlayerMapper 문제점 해결해보기

그래서 다음과 같이 PlayerMapper의 필드를 final로 선언하지 않고 필요한 데이터만 받아서 새로운 객체를 반환할 수 있도록 했다.

 

public class PlayerMapper {
    private int id;
    private Name name;
    private Position position;
    private int racingGameId;
}

이제 필요한 형식에 맞게 새로운 객체를 반환하면 된다.

 

public class PlayerDto {
    private Name name;
    private Position position;

    // 01. PlayerMapper를 인자로 받는 방법
    public PlayerDto(final PlayerMapper playerMapper) {
        this.name = playerMapper.getName();
        this.position = playerMapper.getPosition();
    }

    // 02. 필요한 데이터만 인자로 받는 방법
    public PlayerDto(final Name name, final Position position) {
        this.name = name;
        this.position = position;
    }
}

그러나 이런식으로 진행할 경우 PlayerMapper에 필드가 null인 값을 꺼낼 수도 있는 위험성이 있다.

 

 

🟢 03-04. Mapper를 없애고 DTO만 사용하기

DTO를 사용한다면 Mapper의 단점을 없앨 수 있다.

NPE가 나올 경우가 없어지고 사용하지 않을 데이터를 반환하게될 가능성도 없다.

때문에 Mapper를 지우고 DTO를 이용했다.

public class PlayerDto {
    private String name;
    private int position;

    public PlayerDto(final String name, final int position) {
        this.name = name;
        this.position = position;
    }
}

 

 

 

🟢 03-05. Repository에서 반환하는 DTO를 바깥에서 예상할 수 없게하라 

리뷰어님이 다시 다음과 같이 커멘트를 남겨주셨다.

💁‍♂️ 제 의견을 조심스럽게 말씀드리면 Repository 에서 반환하는 DTO는 바깥에서
어떤 값이 필요할지는 예상할 수도 없고 신경쓸 필요도 없다고 생각해요!
즉 모든 값을 반환해주고 알아서 써라~ 하고 신경 끄는것이 좋다고 생각합니다 :)

 

이 때 여러 궁금한 점이 생겨서 바로 여쭤봤다.

 

🤔 모든 정보를 가질 수 있는 DTO에 지정한 컬럼만 가져올 경우

• 조회문을 지정한 컬럼만 가져오게 할 경우 null이 들어갈 수 있을거 같다.
• 이럴 경우 호출할 수 있으니 값이 있겠지라는 생각으로 NPE를 발생시킬 가능성이 있다.

SELECT id, position, racingGameId FROM PLAYER WHERE racing_game_id = ?

public class PlayerDto {
    private int id; // 1
    private String name; // null
    private int position; // 10
    private int racingGameId; // 1
}

🤔 모든 정보를 가질 수 있는 DTO에 모든 컬럼만 가져올 경우

 

• 만약 모든 컬럼을 가져와서 null 문제를 해결하려고 한다면 필요없는 데이터를 가져와서 비용이 더 나갈 수도 있을거 같다.
• 또한 모든 컬럼을 가져와서 DTO에서 getter를 통해 값을 가져온다면 중요한 정보가 노출될 수도 있을 거 같다.

SELECT * FROM PLAYER WHERE racing_game_id = ?

public class PlayerDto {
    private int id; // 1
    private String name; // "헤나"
    private int position; // 10
    private int racingGameId; // 1
}

 

그리고 아래와 같이 여쭤봤다.

😄 이러한 이유들 때문에 필요한 값만 return 할 수 있도록 DTO를 만들어주는게 더 좋다고 생각이 듭니다.
다만 어떤 행동을 하기 위해서 데이터가 필요할지 미리 알고 진행하기 때문에 DTO를 재사용하기 어렵다는 단점도 있습니다.
찰리는 이러한 경우 어떻게 생각하시는지 궁금합니다. 🤔

 

 

리뷰어님의 답변은 다음과 같았다.

💁‍♂️ 이런 상황을 방지하지 위해 DB에 not null 제약 조건을 걸어주는 방법이 있습니다!
그만큼 테이블 설계도 중요하죠
그리고 null 이 아니어야한다 라는 조건이 있음에도null 값을 DB에 저장하라고 전달되지 않도록 validation 이 잘 되어야겠죠?
null 일 수도 있는 값이다~ 라면 java 에서는 Optional 로 감싸서 표현해볼 수도 있겠구요 :)

public class PlayerDto {
    private int id; // 1
    private Optional<String> name;
    private int position; // 10
    private int racingGameId; // 1

    public String
}

 

사실 모든 데이터를 다 받아오면 그만이다.

하지만 그럴 경우 필요 없는 데이터를 가져오게 되면서 비용이 더 발생할 수도 있다.

이 문제를 해결하기 위해서 PlayerDto에 필드 타입을 Optional로 감싸는 방법이 있다.

 

그렇다고해서 문제가 다 해결되지는 않는다.

Optional로 감싼 값을 사용하기 위해서는 계속해서 확인해줘야 하기 때문이다.

 

아직 내가 생각하는 가장 좋은 방법을 찾지는 못했지만 여러 방식이 있음을 알 수 있었다.

크루들이랑 이야기하면서 더 좋은 방법이 있나 확인하고 다시 블로깅을 이어나가겠다.

 

 

 

 

👊 정리하자면

테스트 검증부는 하드 코딩 하는 편이 좋다.

 

서비스 끼리 의존해도 괜찮을지 고민해봤다.

사용될 수도 있겠지만 나는 단점이 많다 느껴져서 선호하지는 않는다.

 

데이터베이스에서 데이터를 가져오고 저장할 객체를 어떻게 정의해줄 것인가에 대한 정답은 없는 거 같다.

아직 내가 선택한 기준도 없기에 크루들과 이야기해볼 예정이다.