사다리 미션을 수행하던 중, 첫 pr 단계에서 BlockService라는 클래스를 만들어 컨트롤러에서 호출할 수 있는 메소드를 만들어놓으려는 시도를 했다.
public class BlockService {
private static final int HEAD_TO_BLOCK_SIZE = 1;
private static final int HEAD_TO_LEFT_INDEX = 1;
private static final int SECOND_BLOCK_INDEX = 1;
private final PassGenerator generator;
public BlockService(PassGenerator generator) {
this.generator = generator;
}
public Blocks initBlocks(int peopleCount) {
Block firstBlock = new Block(generator.generate());
List<Block> blocks = new ArrayList<>();
blocks.add(firstBlock);
return new Blocks(generateBlocks(peopleCount, blocks));
}
private List<Block> generateBlocks(int peopleCount, List<Block> blocks) {
for (int i = SECOND_BLOCK_INDEX; i < peopleCount - HEAD_TO_BLOCK_SIZE; i++) {
Block leftBlock = blocks.get(i - HEAD_TO_LEFT_INDEX);
Block rightBlock = new Block(generator.generate());
rightBlock = generateBlock(leftBlock, rightBlock);
blocks.add(rightBlock);
}
return blocks;
}
private Block generateBlock(Block leftBlock, Block rightBlock) {
while (rightBlock.isLeftBlockHavePass(leftBlock)) {
rightBlock = new Block(generator.generate());
}
return rightBlock;
}
public Names generateNames(List<String> input){
List<Name> names = new ArrayList<>();
for (String name : input) {
names.add(new Name(name));
}
return new Names(names);
}
}그러나, 첫 리뷰 이후 BlockService라는 클래스가 내 프로그램에서 왜 필요한지에 대해 큰 의문이 들었다. 또 이 클래스 때문에 프로그램의 객체지향이 크게 무너지고 있음을 알 수 있었다.
1. BlockService라는 이름 자체가 매우 위험하다
처음 BlockService라는 클래스를 사용했던 이유는 이러하다.
컨트롤러에서 요청(입력)을 받으면 당연히 무언가 비즈니스 로직을 처리하는 메소드들이 당연히 있어야겠지?
->
그럼 Service라는 계층을 만들어 비즈니스 로직을 몰아넣자
->
컨트롤러에서는 Service 클래스의 몇 가지 메소드를 호출하는 것 만으로 역할을 완료할 수 있을거야
깊은 고민 없이 당연히 mvc 패턴을 써야하겠지? 라는 생각에 BlockService라는 클래스를 만들고 사용했다.
그러나, BlockService라는 네이밍을 가진 객체를 만든 순간부터, 남들이 내 코드를 보면 "아 모든 비즈니스 로직은 이 객체에 총괄되어 있구나"라는 생각을 할 수 밖에 없게 된다.
즉, 객체의 협력으로 문제를 해결하는 것이 아닌, 높은 결합도를 가진 클래스 하나가 모든 문제를 해결한다. 이 순간 객체지향이 무너질 위험이 생긴다.
2. 객체가 행동할 줄을 모른다
위 1번과 비슷한 맥락에서 문제점을 파악할 수 있다. 첫 pr 코드, 그러니까 BlockService 클래스가 존재하던 코드에서 도메인 객체의 모습은 다음과 같았다
//Block.java
public class Block {
private final boolean pass;
public Block(boolean pass) {
this.pass = pass;
}
public boolean isLeftBlockHavePass(Block leftBlock) {
return leftBlock.pass && this.pass;
}
public boolean isPass() {
return pass;
}
}//Blocks.java
public class Blocks {
private final List<Block> blocks;
public Blocks(List<Block> blocks) {
this.blocks = blocks;
}
public List<Block> getBlocks() {
return blocks;
}
}Block이라는 객체가 할 수 있는 일은
- 본인이 이동할 수 있는 통로가 있는지 인지할 수 있다
- 다른 Block이 통로를 가지고 있는지 인지할 수 있다
딱 두 가지다. 그러나 이마저도 "행동"의 관점에서 부합하는 역할이 보이지 않는다.
Blocks의 경우에는 더 심각하다. 단순히 리스트를 감싸는 것 이외에는 아무런 역할을 하지 않고 있다.
심지어 BlockService 클래스를 보면
...
public Blocks initBlocks(int peopleCount) {
Block firstBlock = new Block(generator.generate());
List<Block> blocks = new ArrayList<>();
blocks.add(firstBlock);
return new Blocks(generateBlocks(peopleCount, blocks));
}
private List<Block> generateBlocks(int peopleCount, List<Block> blocks) {
for (int i = SECOND_BLOCK_INDEX; i < peopleCount - HEAD_TO_BLOCK_SIZE; i++) {
Block leftBlock = blocks.get(i - HEAD_TO_LEFT_INDEX);
Block rightBlock = new Block(generator.generate());
rightBlock = generateBlock(leftBlock, rightBlock);
blocks.add(rightBlock);
}
return blocks;
}
...Blocks를 생성하고, Blocks를 초기화하는 책임이 BlockService에 담겨있다. 객체가 본인 스스로의 상태에 대해 아무런 관리를 하지 못하는 웃픈 상황이 발생한다.
사다리 미션의 요구사항에 "일급 컬렉션을 사용하라" 라는 말이 있었다.
하나의 객체를 컬렉션 안에서 병렬적으로 관리할 때는, 당연히 하나의 객체로 감싸줘야겠지?
위와 같은 생각 때문에 아무 고민 없이 "당연하게" Blocks라는 객체를 만들고 사용했다. 그러나 정작 클래스를 만들고 나니, Blocks가 하는 일이 전혀 없어서 리뷰어님에게 "단순히 리스트를 감싸기만 하는 경우에도 일급컬렉션이 필요한가요?" 라는 질문을 하게 되었다.
답은 스스로에게서 찾을 수 있었다.
당연한 코드 구조는 없다. 항상 스스로 필요성을 느끼고 코드 구조를 만들어가야 한다.
"당연하게" service라는 클래스를 만들고 사용했기에 객체의 역할이 몰려 결합도가 높아졌고, "당연하게" 일급 컬렉션을 사용했기에 아무런 행동도 못하는 바보같은 객체가 탄생했다. 객체지향이 와르르 무너졌다.
해결한 과정
BlockService라는 클래스를 과감하게 없애고, 객체들에게 더 역할을 부여해보도록 코드를 짜보았다
//Block.java 변경
public class Block {
private final Pass pass;
public Block(Pass pass) {
this.pass = pass;
}
public static Block buildFirstBlock(boolean generatedPass) {
if (generatedPass) {
return new Block(Pass.RIGHT);
}
return new Block(Pass.NOTHING);
}
public static Block buildMiddleBlock(Block leftBlock, boolean generatedPass) {
if (leftBlock.pass == Pass.RIGHT) {
return new Block(Pass.LEFT);
}
if (!generatedPass) {
return new Block(Pass.NOTHING);
}
return new Block(Pass.RIGHT);
}
public static Block buildLastBlock(Block leftBlock) {
if (leftBlock.pass == Pass.RIGHT) {
return new Block(Pass.LEFT);
}
return new Block(Pass.NOTHING);
}
public Pass getPass() {
return pass;
}
}먼저 변경된 Block의 모습이다. 이제 Block은 정적 메소드를 통해, 각 상황마다 적절한 Block을 초기화하는 능력을 갖추게 되었다. Block이 한 Line에서 처음 생성되는 경우인지, 중간인지, 맨 마지막인지에 따라 다른 메소드를 사용해 Block을 초기화할 수 있고, 어떤 값이 들어오느냐에 따라 알맞은 형태의 Block을 초기화할 수 있다.
//Blocks.java -> Line.java로 변경
public class Line {
private final List<Block> line = new ArrayList<>();
public Line(int personCount, PassGenerator passGenerator) {
validateMinPerson(personCount);
line.add(Block.buildFirstBlock(passGenerator.generate()));
while (personCount-- > 2) {
line.add(Block.buildMiddleBlock(getLeftBlock(), passGenerator.generate()));
}
line.add(Block.buildLastBlock(getLeftBlock()));
}
private Block getLeftBlock() {
return line.get(line.size() - 1);
}
private void validateMinPerson(int personCount) {
if (personCount < 2){
throw new IllegalArgumentException("참여인원은 최소 두명이어야 합니다");
}
}
public List<String> getLineBlockPass() {
return line.stream()
.map(block -> block.getPass().toString())
.collect(Collectors.toList());
}
}변경된 Blocks의 모습은 다음과 같다. 먼저 사다리 게임의 특성에 맞게 "한 줄"이라는 개념의 네이밍이 Blocks 보다는 Line이 더 적절해 보였기에 변경했다.
이제 Line은 감싸고 있는 Block의 정적 메소드를 스스로 알맞게 활용하여 본인의 컬렉션에 담아 관리할 수 있으며, 검증 처리 또한 수행할 수 있다.
느낀 점
위에도 언급하였듯 모든 코드의 형태와 구조에는 "당연한 것"이 없다는 것을 느꼈다. 무엇이든 필요성과 당위가 있기에 등장한 것이고 그것은 본인이 코드를 짜면서 직접 불편함을 느끼고 필요성을 찾아가야 하는 것 같다.
따라서 어떤 틀에 박힌 상태로 코드를 짜는 순간 코드가 고착화되고, 객체지향이 무너지는 결과가 초래될 수 있다고 생각했다.
그렇기에 TDD라는 개념이 중요하지 않을까라는 생각도 들었다. 본인이 어떠한 도메인에 대한 이해가 충분히 생긴 상태에서 코드 구조를 확장시켜 나가야지, 도메인에 대한 이해도와 지식이 없는 상태에서 틀을 짠다는 것은 충분히 위험한 선택일 것이다.
