"시점 이동 줄이기" 페이지의 내용에 의견이 있습니다 #37
Replies: 3 comments
-
후자 예시를 잘 못 가져오신 것 같네요.
|
Beta Was this translation helpful? Give feedback.
-
제가 "후자"라는 단어를 오해하게끔 글을 작성한것같아요. 여기서 "후자"는 해당 페이지에서의 예시중 개선된 코드를 일컫는 것이었습니다. 첨부한 코드는 모두 첨부한 페이지 에서의 개선전 코드를 예시로든게 맞습니다. // A
function Page() {
const user = useUser();
const policy = getPolicyByRole(user.role);
return (
<div>
<Button disabled={!policy.canInvite}>Invite</Button>
<Button disabled={!policy.canView}>View</Button>
</div>
);
}
// B
function Page() {
const user = useUser();
const policy = {
admin: { canInvite: true, canRead: true },
viewer: { canInvite: false, canRead: true },
}[user.role];
return (
<div>
<Button disabled={!policy.canInvite}>Invite</Button>
<Button disabled={!policy.canRead}>Read</Button>
</div>
);
} 이정도 양과 복잡도라면 B의 코드가 좋다고 생각하지만 // A
function Page() {
const user = useUser();
const policy = getPolicyByRole(user.role);
return (
<div>
<Button disabled={!policy.canInvite}>Invite</Button>
<Button disabled={!policy.canView}>View</Button>
</div>
);
}
function getPolicyByRole() {
...
}
// B
function Page() {
const user = useUser();
const policy = (() => {
...
if(!user.hasPolicy) return { ~~ }
if(user.policy === 'A') return { ~~ }
if(user.policy === 'B') {
return { ~~ }
} else {
if(user.hello === 'world'){ return { ~~ } }
}
...
})()
return (
<div>
<Button disabled={!policy.canInvite}>Invite</Button>
<Button disabled={!policy.canRead}>Read</Button>
</div>
);
} policy가 어떻게 구해지는지에 관심이 없는 사람은 B에 비해 A의 케이스가 읽기 쉽습니다. 하지만 말씀하신걸 듣고보니 "시점 이동" 자체에 대해 논하는것과는 다른 논점일 수 있겠네요. 시점 이동을 감안해야 할때가 있다는 것이고 시점 이동은 항상 가독성에 좋지못하다는것에 동의합니다. 의견 감사해요. |
Beta Was this translation helpful? Give feedback.
-
문서의 내용처럼 저도 이 의견엔 동의해요! 하지만 @ho1234c 님께서 남겨주신 의견처럼 그 복잡도가 올라갈 경우는 시점 이동을 감안해야 한다는 점에도 공감하고 있어서 의견 남겨보아요. const policy = (() => {
...
if(!user.hasPolicy) return { ~~ }
if(user.policy === 'A') return { ~~ }
if(user.policy === 'B') {
return { ~~ }
} else {
if(user.hello === 'world'){ return { ~~ } }
}
...
})() 위에 예시로 들어주신 정책 / 권한을 정하는 로직이 복잡해 질수록 별도의 함수로 추상화하여 함수의 이름에서 의도를 드러내어 그 결과물을 충분히 예측할 수 있다면 꼭 읽는 순서에 내부 구현까지 포함할 필요는 없다고 생각해요. 예를 들어 제가 담당하지 않은 서비스 / 도메인의 코드리뷰를 하게 될 때, 복잡한 계산이나 비지니스로직, 혹은 해당 예시처럼 서비스에서의 권한 / 정책까지 다 이해하며 코드를 읽는 것보다 "이건 이 서비스가 가진 정책이구나" 라고 간단히 파악할 수 있는 코드가 더 읽기 쉬웠어요. 리뷰의 대상 PR이 로직을 어떻게 구현할 것인가? 라면 이야기는 달라지겠지만, 그게 아닐 경우 이를 세부 구현이라 생각하고 넘어간 뒤, 작성자가 놓칠 수 있는 설계적인 관점 등에 시간을 더 투자할 수도 있을 것 같아요. 만약 권한 / 정책이 어떻게 결정되는지 로직이 더욱 복잡해지더라도 시점 이동을 고려하여 드러내고 싶다면, ts-pattern 같은 라이브러리를 활용해서 패턴 매칭으로 가독성을 올리는 방법도 선택할 수 있을 것 같아요! |
Beta Was this translation helpful? Give feedback.
-
시점 이동이 코드의 가독성을 저해한다는건 사실이지만 시점 이동을 할지 말지 선택권을 준다 라는 측면에서는 개선 전의 코드가 장점있다는 생각이 들어요
이 페이지의 예시중
이 코드를 읽는 목적이 러프하게
Page
라는 컴포넌트의 책임을 파악하기 위함이라면 세부사항은 생략하고 읽기마련입니다.getPolicyByRole
라는 함수의 이름히 적절히 지어졌기 때문에 여기까지만 읽어도 "user의 role에 따라 버튼을 선택적으로 렌더링하는 컴포넌트구나"라는 사실은 충분히 파악할 수 있습니다. 러프하게 코드를 읽는 상황이라면 그 아래 코드는 읽지 않아도 됩니다.개선된 코드에서는 user의 role이 admin이면 이렇게, view 라면 저렇게 한다 라는 코드의 동작이 잘 드러나고 있기 때문에 비교적 코드가 길어지고 생략해서 읽기에는 불편합니다.
그래서 전자의 코드는 "얼마나 자세히 읽을지를 읽는 사람이 결정할 수 있다"는 측면에서 후자의 코드보다 더 나을 수 있다고 생각해요. 즉, 코드를 읽는 어떤 사람에게는 아래쪽 내용이 코드의 세부사항이 될 수 있습니다.
물론 이 코드 조각에서는 예시의 코드 블럭 크기가 작기 때문에 인지적 관점에서 큰 차이가 느껴지지 않으나 코드의 복잡도가 높아질수록, 추상화가 더 복잡하게 이루어져야하는 상황일수록 세부사항을 생략하는게 가독성 관점에서 좋은 경우가 많아질거라 생각합니다.
Beta Was this translation helpful? Give feedback.
All reactions