연동 채널마다 처리 코드가 따로 있었다. 여덟 개였고 서로 비슷했다.
Table of contents
Open Table of contents
여덟 개가 비슷했다
파일 목록만 봐도 같은 모양이었다.
ChannelAHandler.php 412줄
ChannelBHandler.php 398줄
ChannelCHandler.php 445줄
...
하나를 열어 보니 흐름이 같았다.
public function process(array $raw): Result {
$this->validate($raw);
$order = $this->map($raw);
$this->checkDuplicate($order);
$this->save($order);
$this->notify($order);
return Result::ok();
}
여덟 개가 전부 validate 부터 notify 까지 이 다섯 단계였고 안의 내용만 달랐다. 비슷하니까 하나로 합치면 되겠다는 생각이 먼저 들었다.
그런데 비슷하다는 인상만으로 합치면 실제로 달라야 하는 부분까지 뭉개진다. 합치기 전에 무엇이 같고 무엇이 다른지를 세어 보기로 했다.
비슷하다는 것은 눈으로 본 인상이지 diff 로 잰 값이 아니다. 그 인상으로 설계를 정하면 나중에 조건문으로 돌아온다.
여덟 개가 비슷한 것과 같아야 하는 것은 다른 말이었고 앞은 관찰이며 뒤는 판단이다.
무엇이 같고 무엇이 다른지 세어 봤다
단계별로 여덟 개를 나란히 놓고 한 줄씩 비교했다.
단계 같은 곳 다른 곳
validate 3 5
map 0 8 전부 다르다
checkDuplicate 8 0 전부 같다
save 7 1
notify 6 2
map 은 채널마다 필드 이름이 달라서 전부 달랐고 checkDuplicate 는 여덟 개가 글자까지 같았다.
전부 같은 것과 전부 다른 것이 한 파일 안에 섞여 있었다. 그러면 합칠지 말지를 파일 단위가 아니라 단계 단위로 정해야 한다.
ChannelAHandler 를 통째로 합치거나 통째로 두는 선택지만 놓으면 어느 쪽도 안 맞았다. 한 파일 안에 성격이 다른 다섯 단계가 들어 있기 때문이다.
세어 보기 전에는 그 사실이 안 보였고 표를 만들고 나서야 단계마다 다른 결정이 필요해졌다.
조치 — 같은 것부터 뽑기
checkDuplicate 가 여덟 벌 있었으므로 하나만 남기고 지웠다.
abstract class ChannelHandler {
protected function checkDuplicate(Order $o): void {
if ($this->repo->existsByChannelOrderNo($o->channel, $o->channelOrderNo)) {
throw new DuplicateOrder($o->channelOrderNo);
}
}
}
한 줄을 고치면 여덟에 반영된다. 전에는 여덟 곳을 고쳐야 했는데 실제로 한 곳이 빠져 있었다.
// ChannelF 만 조건이 하나 적었다
if ($this->repo->existsByChannelOrderNo($o->channelOrderNo)) { // channel 을 안 본다
ChannelF 만 channel 인자를 안 넘기고 있었다. 채널이 달라도 주문번호가 같으면 중복으로 보고 막고 있었던 것이다.
같아야 하는 것이 여덟 벌 있으면 그중 하나는 다르다. 합치는 작업 자체가 그 어긋남을 찾아 주는 셈이었다.
여덟 곳을 똑같이 고치는 일이 반복되면 언젠가 한 곳이 빠지고 그것은 조용히 다르게 돈다.
이 어긋남은 코드를 읽어서는 안 나오는데 같은 것을 여덟 번 읽으면 세 번째부터 같아 보인다.
다른 것은 데이터로 뺐다
map 은 전부 달랐지만 자세히 보니 규칙이 있었다.
protected function fieldMap(): array {
return [
'orderNo' => 'order_id',
'buyerName' => 'buyer.name',
'amount' => 'payment.total',
];
}
어느 필드가 어디에 있는지만 다르고 옮기는 방식은 여덟 개가 같았다. 그러면 표만 채널별로 두고 옮기는 코드는 하나면 된다.
protected function map(array $raw): Order {
$o = new Order();
foreach ($this->fieldMap() as $to => $from) {
$o->$to = Arr::get($raw, $from);
}
return $o;
}
fieldMap 이 내놓는 표를 돌면서 Arr::get 으로 옮긴다. 새 채널이 생기면 그 표만 쓰면 된다.
다르다고 해서 전부 코드로 둘 이유는 없고 다른 것이 값이면 값으로 절차면 코드로 둔다.
map 단계는 다른 것이 필드 이름뿐이라 값 쪽이었고 표 한 장이 코드 400줄을 대신했다.
판단 기준 — 묶지 않을 자리
validate 는 표로 못 만들었다. 채널마다 검사 내용이 아예 달랐기 때문이다.
한 곳은 사업자번호를 보고 한 곳은 배송지 형식을 봤는데 공통점 없는 것을 억지로 묶으면 if 만 늘어난다.
abstract protected function validate(array $raw): void;
그래서 abstract 로 선언만 해 두고 내용은 각자 쓰게 뒀다. 전부 묶는 것이 목적이 아니었고 같은 것만 묶고 다른 것은 다른 채로 두는 것이 맞았다.
결과 — 줄어든 것
정리 전후를 세어 봤다.
전 8개 파일 3,412줄
후 공통 380줄 + 채널별 평균 120줄 x 8 = 1,340줄
wc -l 로 세니 절반 이하가 됐는데 줄 수보다 중요한 것이 따로 있었다.
전 기존 것을 복사해서 고친다. 3일
후 표와 검사만 쓴다. 반나절
새 채널을 붙이는 데 걸리는 시간이 사흘에서 반나절이 됐다. 복사해서 고치는 방식은 매번 전체를 다시 읽어야 했다.
복사본은 원본이 무엇을 하는지 다 읽어야 고칠 수 있는데 물려받으면 쓸 두 가지만 알면 된다.
줄 수가 준 것은 결과이고 읽을 양이 준 것이 이유인데 붙이는 시간이 그 차이를 보여 줬다.
검증 — 새 채널 붙이기
정리한 뒤에 실제로 새 채널을 붙여 봤다.
class ChannelIHandler extends ChannelHandler {
protected function fieldMap(): array { return [...]; }
protected function validate(array $raw): void { ... }
}
fieldMap 과 validate 둘만 쓰면 됐고 나머지는 ChannelHandler 에서 물려받는다. 그런데 붙이면서 공통 쪽에 빠진 것이 하나 나왔다.
새 채널만 금액에 소수점이 있었다.
protected function amountScale(): int { return 0; } // 기본은 정수
기본값을 두고 필요한 채널만 바꾸게 했다. 이런 것은 코드를 아무리 봐도 안 나오고 실제로 붙여 봐야 나온다.
정리
- 비슷한 처리기가 여럿이면 단계별로 같은 곳과 다른 곳을 센다
- 비슷하다는 인상만으로 합치면 달라야 할 부분까지 뭉개진다
- 전부 같은 것과 전부 다른 것이 한 파일에 섞여 있다
- 합칠지는 파일 단위가 아니라 단계 단위로 정한다
- 같아야 하는 것이 여덟 벌이면 그중 하나는 다르고 합치면서 그것이 드러난다
- 다른 것 중에서도 규칙이 있으면
fieldMap같은 표로 빼고 코드는 하나로 둔다 - 공통점이 없는 것은
abstract로 두고 각자 쓰게 한다 - 줄 수보다 새 채널을 붙이는 시간이 사흘에서 반나절이 된 것이 컸다
- 정리한 뒤 실제로 붙여 봐야
amountScale같은 빠진 것이 나온다