원문: https://profy.dev/article/react-junior-code-review-and-refactoring-1
주니어 React 개발자 혹은 지망하는 사람으로서 좋은 코드인지, 어떻게 개선할 수 있는지 이해하기 어렵습니다. 숙련된 개발자에게 리뷰를 받으면 좋을 것입니다. 하지만 코드를 공유하려면 용기가 필요합니다. 그리고 온라인 커뮤니티에서는 종종 얕은 수준의 리뷰를 받게 됩니다.
최근 한 개발자가 용감하게 코드 리뷰를 요청했습니다. 칭찬합니다. 기능이 흥미로웠고, 코드도 나쁘지 않았습니다. 그러나 주니어 React 개발자들이 하는 매우 흔한 실수가 몇가지 있었습니다.
이 실수들로부터 배울 수 있는 기회가 생겼습니다. 이 글에서, 코드 리뷰 및 단계적인 리팩터링의 과정을 찾을 수 있습니다.
- 더 나은 데이터 구조를 사용
- 불필요한 상태 제거
- 몇 가지 다른 리팩터링 및 정리
우리는 코드의 성능을 향상시키고, 코드 읽기와 유지보수를 더 쉽게 하며, 컴포넌트를 초기 177줄에서 102줄로 단축할 것입니다. (좋은 측정 기준은 아닙니다.)
여러분이 동영상을 선호한다면 아래 동영상에서 전체 리뷰 및 리펙터링 세션을 시청할 수 있습니다.
메모: 이 페이지의 코드는 Map 대신 Set을 이용하는 동영상의 코드에 비해 약간 개선되었습니다.
리뷰할 컴포넌트
기능
컴포넌트는 약간 복잡한 기능을 가진 테이블입니다. 다음은 간단한 동영상입니다.
테이블은 이슈 목록을 렌더링합니다. 문제는 마지막 열에 "open" 또는 "resolved"로 표시되는 상태입니다. "resolved" 상태의 문제는 선택할 수 없습니다.
모든 행이 아닌 일부 행을 선택하면 상단의 체크박스는 부분적으로 선택됩니다. 이 체크박스를 클릭하면 사용자가 "open" 상태의 모든 이슈를 선택하거나 선택 취소할 수 있습니다.
원본 코드
원본 코드는 나쁘지 않습니다. 정상적으로 동작합니다. 대부분은 꽤 읽기 좋습니다. 특히 높은 수준에서 말이죠. 그리고 이 코드는 많은 주니어 개발자들이 작성하는 리액트 작성법과 유사하고 동시에 몇 가지 문제점을 갖고 있습니다. 우리는 그 문제들에 대해 아래에서 논의할 것입니다.
가장 먼저 할 일은 코드를 살펴보는 일이고 여기에서 코드를 확인 할 수 있습니다. 이 CodeSandbox에서 작동하는 코드를 볼 수 있습니다.
import { useState } from "react";
import classes from "./Table.module.css";
function Table({ issues }) {
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({
checked: false,
backgroundColor: "#ffffff",
})
);
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
const handleOnChange = (position) => {
const updatedCheckedState = checkedState.map((element, index) => {
if (position === index) {
return {
...element,
checked: !element.checked,
backgroundColor: element.checked ? "#ffffff" : "#eeeeee",
};
}
return element;
});
setCheckedState(updatedCheckedState);
const totalSelected = updatedCheckedState
.map((element) => element.checked)
.reduce((sum, currentState, index) => {
if (currentState) {
return sum + issues[index].value;
}
return sum;
}, 0);
setNumCheckboxesSelected(totalSelected);
handleIndeterminateCheckbox(totalSelected);
};
const handleIndeterminateCheckbox = (total) => {
const indeterminateCheckbox = document.getElementById(
"custom-checkbox-selectDeselectAll"
);
let count = 0;
issues.forEach((element) => {
if (element.status === "open") {
count += 1;
}
});
if (total === 0) {
indeterminateCheckbox.indeterminate = false;
setSelectDeselectAllIsChecked(false);
}
if (total > 0 && total < count) {
indeterminateCheckbox.indeterminate = true;
setSelectDeselectAllIsChecked(false);
}
if (total === count) {
indeterminateCheckbox.indeterminate = false;
setSelectDeselectAllIsChecked(true);
}
};
const handleSelectDeselectAll = (event) => {
let { checked } = event.target;
const allTrueArray = [];
issues.forEach((element) => {
if (element.status === "open") {
allTrueArray.push({ checked: true, backgroundColor: "#eeeeee" });
} else {
allTrueArray.push({ checked: false, backgroundColor: "#ffffff" });
}
});
const allFalseArray = new Array(issues.length).fill({
checked: false,
backgroundColor: "#ffffff",
});
checked ? setCheckedState(allTrueArray) : setCheckedState(allFalseArray);
const totalSelected = (checked ? allTrueArray : allFalseArray)
.map((element) => element.checked)
.reduce((sum, currentState, index) => {
if (currentState && issues[index].status === "open") {
return sum + issues[index].value;
}
return sum;
}, 0);
setNumCheckboxesSelected(totalSelected);
setSelectDeselectAllIsChecked((prevState) => !prevState);
};
return (
<table className={classes.table}>
<thead>
<tr>
<th>
<input
className={classes.checkbox}
type={"checkbox"}
id={"custom-checkbox-selectDeselectAll"}
name={"custom-checkbox-selectDeselectAll"}
value={"custom-checkbox-selectDeselectAll"}
checked={selectDeselectAllIsChecked}
onChange={handleSelectDeselectAll}
/>
</th>
<th className={classes.numChecked}>
{numCheckboxesSelected
? `Selected ${numCheckboxesSelected}`
: "None selected"}
</th>
</tr>
<tr>
<th />
<th>Name</th>
<th>Message</th>
<th>Status</th>
</tr>
</thead>
<tbody>
{issues.map(({ name, message, status }, index) => {
let issueIsOpen = status === "open";
let onClick = issueIsOpen ? () => handleOnChange(index) : null;
let stylesTr = issueIsOpen
? classes.openIssue
: classes.resolvedIssue;
return (
<tr
className={stylesTr}
style={checkedState[index]}
key={index}
onClick={onClick}
>
<td>
{issueIsOpen ? (
<input
className={classes.checkbox}
type={"checkbox"}
id={`custom-checkbox-${index}`}
name={name}
value={name}
checked={checkedState[index].checked}
onChange={() => handleOnChange(index)}
/>
) : (
<input
className={classes.checkbox}
type={"checkbox"}
disabled
/>
)}
</td>
<td>{name}</td>
<td>{message}</td>
<td>
{issueIsOpen ? (
<span className={classes.greenCircle} />
) : (
<span className={classes.redCircle} />
)}
</td>
</tr>
);
})}
</tbody>
</table>
);
}
export default Table;테이블에 렌더링되는 이슈 배열은 다음과 같습니다.
[
{
"id": "c9613c41-32f0-435e-aef2-b17ce758431b",
"name": "TypeError",
"message": "Cannot read properties of undefined (reading 'length')",
"status": "open",
"numEvents": 105,
"numUsers": 56,
"value": 1
},
{
"id": "1f62d084-cc32-4c7b-943d-417c5dac896e",
"name": "TypeError",
"message": "U is not a function",
"status": "resolved",
"numEvents": 45,
"numUsers": 34,
"value": 1
},
...
]컴포넌트는 177줄의 코드(LOC)로 상당히 복잡한 편입니다. 하지만 앞서 말씀드렸듯이, 높은 수준에서는 꽤 읽기 쉽습니다. 아래 코드는 함수들을 축소했을 때 보여지는 코드입니다.
function Table({ issues }) {
// checkbox들이 선택되어 있는지 결정합니다.
const [checkedState, setCheckedState] = useState(...);
// 상단의 checkbox가 선택되어 있는지 결정합니다.
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
// 선택된 체크박스들의 숫자를 나타냅니다.
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
// 행 또는 체크박스의 클릭을 처리합니다.
const handleOnChange = (position) => { ... };
// 상단 체크박스의 부분적으로 체크된 상태 설정을 담당합니다.
const handleIndeterminateCheckbox = (total) => { ... };
// 상단의 체크박스의 클릭을 처리합니다.
const handleSelectDeselectAll = (event) => { ... };
return (
<table>
...
</table>
);
}나쁘지 않습니다. 하지만 개선의 여지가 있습니다.
원본 코드의 문제
문제 1: 차선의 데이터 구조
컴포넌트의 상단을 살펴보겠습니다.
function Table({ issues }) {
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);
...checkedState변수는 배열입니다.- 이슈 배열과 동일한 길이로 초기화되었습니다.
- 각 항목은 이슈의 선택된 상태를 나타내는 객체입니다.
첫 번째 문제는 객체 배열이 항상 변형되기 어렵다는 것입니다. 배열 내부의 인덱스를 통해 올바를 객체에 접근해야 합니다. 그래서 우리는 인덱스를 찾아서 전달해야 합니다.
두 번째 문제는 이 접근 방식이 확장성이 좋지 않다는 것입니다. 예를 들어, 만 개의 이슈가 있는 경우 대부분 false로 채워진 만 개의 checkedState 배열을 가지게 됩니다.
그 대안은 다음과 같습니다. 이슈 id와 선택되어 있는 값을 매핑하는 데이터 구조인 객체 또는 Map을 사용할 수 있습니다.
// 대안 데이터 구조 (두 개의 체크박스가 선택되었을 때의 예제)
{
"issue-id-1": true,
"issue-id-4": true,
}더 좋은 것은 자바스크립트에는 배열과 유사하지만 고유한 값만 보유할 수 있는 네이티브 Set를 가지고 있으며 성능적인 방식으로 접근하도록 최적화되어 있다는 것입니다.
문제 2: 상태로부터 변수를 파생하지 않음
다시 컴포넌트의 상단을 보면, 우리는 또 다른 주니어 개발자의 코드에서 발생하는 일반적인 문제를 확인할 수 있습니다. 바로 props나 다른 상태에서 파생될 수 있는 불필요한 상태 변수입니다.
function Table({ issues }) {
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
...이 코드에는 세 가지의 불필요한 상태가 있습니다.
backgroundColor는checked의 값에서 파생될 수 있는 스타일입니다. (체크 표시가 참이면#eee, 아니면#fff입니다.)selectDeselectAllIsChecked는 상단에 있는 체크박스에 대한checked상태를 설정하는 데 사용됩니다. (checkedState와issues에서 파생될 수 있습니다.)numCheckboxesSelected는 해당 체크박스 옆에 있는 숫자입니다. (checkedState에서 파생될 수 있습니다.)
문제 3: DOM에 직접 접근
코드를 조금 더 내려보면, 우리는 상단 체크박스의 indeterminate 상태를 처리하는 함수를 찾을 수 있습니다.
function Table({ issues }) {
...
const handleIndeterminateCheckbox = (total) => {
const indeterminateCheckbox = document.getElementById(
"custom-checkbox-selectDeselectAll"
);
let count = 0;
issues.forEach((element) => {
if (element.status === "open") {
count += 1;
}
});
if (total === 0) {
indeterminateCheckbox.indeterminate = false;
setSelectDeselectAllIsChecked(false);
}
if (total > 0 && total < count) {
indeterminateCheckbox.indeterminate = true;
setSelectDeselectAllIsChecked(false);
}
if (total === count) {
indeterminateCheckbox.indeterminate = false;
setSelectDeselectAllIsChecked(true);
}
};
...상단 체크박스가 document.getElementById(...)를 통해 접근되는 것을 확인할 수 있습니다. 이는 React 앱에서 비교적 드물게 사용되며 대부분 필요하지 않습니다.
DOM 요소에 직접 indeterminateCheckbox.indeterminate을 설정해야 합니다. 우리는 체크박스에 prop으로 indeterminate 설정할 수 없습니다.
문제 4: 변수 이름
이 코드의 작성자는 변수와 함수에 대한 좋은 이름을 선택하는 데 약간의 노력을 기울였습니다. 칭찬합니다.
불행히도, 몇 가지 이름은 혼란을 야기합니다. 컴포넌트 상단에서 이에 대한 몇 가지 예시를 찾을 수 있습니다.
function Table({ issues }) {
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
...예를 들어, checkedState는 중복되고 오해의 소지가 있습니다. 저는 여기서 부울을 기대했지만 그것은 객체 배열입니다. 더 좋은 이름은 checkedItems 일 수 있습니다.
selectDeselectAllIsChecked는 이해는 되지만 읽기 조금 어렵습니다. 모든 체크박스가 체크되었는지 안되었는지 말한다는 것을 5번 정도 읽어야 했습니다. isEveryItemChecked가 더 좋은 이름일 수 있습니다.
리팩터링
이제 원본 코드의 몇 가지 문제를 분석했으므로 단계별로 리팩터링을 시작해 보겠습니다.
단계 1: 데이터 구조를 배열에서 SET으로 변경
가장 효과적인 단계는 배열 대신 Set로 checkedState를 사용하는 것입니다. 객체로 사용할 수도 있었지만, Set는 우리가 꽤 많이 사용하는 size에 접근하는 것과 같은 약간의 이점이 있습니다. Map과 비교하면 키-값 대신 선택한 모든 항목의 id만 알면 되기 때문에 Set이 약간 더 적합합니다.
우리는 이미 이 코드를 수정하면서 상태에서 backgroundColor를 지우고 상태 변수의 이름을 변경했습니다.
//before
const [checkedState, setCheckedState] = useState(
new Array(issues.length).fill({ checked: false, backgroundColor: "#ffffff" })
);
// after
const [checkedById, setCheckedById] = useState(new Set());다시 한 번, checkedById 상태의 모양(배열로 표시 됨)을 알려드립니다. 다음과 같습니다.
// checkedById의 모양 (두 개의 체크박스가 선택되었을 때)
["issue-id-1", "issue-id-4"]이 변화는 사소한 것이 아닙니다. 우리의 나머지 부분의 코드에 영향을 미칩니다. 코드의 각 부분이 어떻게 영향을 받는지 하나씩 알아보겠습니다.
테이블의 체크박스에 연결된 핸들러 기능부터 시작하겠습니다.
다음은 전, 후 코드입니다.
// before
const handleOnChange = (position) => {
const updatedCheckedState = checkedState.map((element, index) => {
if (position === index) {
return {
...element,
checked: !element.checked,
backgroundColor: element.checked ? "#ffffff" : "#eeeeee",
};
}
return element;
});
setCheckedState(updatedCheckedState);
const totalSelected = updatedCheckedState
.map((element) => element.checked)
.reduce((sum, currentState, index) => {
if (currentState) {
return sum + issues[index].value;
}
return sum;
}, 0);
setNumCheckboxesSelected(totalSelected);
handleIndeterminateCheckbox(totalSelected);
};
// after
const handleOnChange = (id) => {
const updatedCheckedById = new Set(checkedById);
if (updatedCheckedById.has(id)) {
updatedCheckedById.delete(id);
} else {
updatedCheckedById.add(id);
}
setCheckedById(updatedCheckedById);
const totalSelected = updatedCheckedById.size;
setNumCheckboxesSelected(totalSelected);
handleIndeterminateCheckbox(totalSelected);
};우리는 꽤 많은 어수선한 코드를 정리했습니다. 다음과 같이 요약할 수 있습니다.
- 원본 코드의
return sum + issues[index].value줄이 흥미롭습니다. 데이터의 값을 사용하여 합계를 증가시키고 선택한 요소의 수를 계산합니다. 이건 너무 복잡한 것 같습니다. - 이제 리팩터링된 코드에서
updatedCheckedById.size를 사용하여 선택한 요소의 수를 얻을 수 있습니다. - 우리는 상태에서
false값을 원하지 않기 때문에 체크박스 체크를 해제하면updatedCheckedById.delete(id)를 통해 해당 값을 삭제합니다.
상단 체크박스에 연결된 핸들러 기능을 계속 진행해 보겠습니다.
다음은 전, 후 코드입니다.
// before
const handleSelectDeselectAll = (event) => {
let { checked } = event.target;
const allTrueArray = [];
issues.forEach((element) => {
if (element.status === "open") {
allTrueArray.push({ checked: true, backgroundColor: "#eeeeee" });
} else {
allTrueArray.push({ checked: false, backgroundColor: "#ffffff" });
}
});
const allFalseArray = new Array(issues.length).fill({
checked: false,
backgroundColor: "#ffffff",
});
checked ? setCheckedState(allTrueArray) : setCheckedState(allFalseArray);
const totalSelected = (checked ? allTrueArray : allFalseArray)
.map((element) => element.checked)
.reduce((sum, currentState, index) => {
if (currentState && issues[index].status === "open") {
return sum + issues[index].value;
}
return sum;
}, 0);
setNumCheckboxesSelected(totalSelected);
setSelectDeselectAllIsChecked((prevState) => !prevState);
};
// after
const handleSelectDeselectAll = (event) => {
if (event.target.checked) {
const openIssues = issues.filter(({ status }) => status === "open");
const allChecked = new Set(openIssues.map(({ id }) => id));
setCheckedById(allChecked);
setNumCheckboxesSelected(allChecked.size);
} else {
setCheckedById(new Set());
setNumCheckboxesSelected(0);
}
setSelectDeselectAllIsChecked((prevState) => !prevState);
};다시 다음은 요약입니다.
- 원래 구현은
allTrueArray와allFalseArray로 약간 번거롭습니다. 우리는 이것도 리팩터링을 할 수 있지만 객체를 사용하기 때문에 간단하게 제거할 수 있습니다. - 새로운 구현에서 Set을 사용하기 때문에 새로운
allChecked상태를 만드는 것은 매우 간단합니다. 열려 있는 모든 이슈를 필터링한 다음 id 배열에서 new Set를 생성합니다.
마지막으로 조정해야 할 코드는 컴포넌트에서 반한되는 JSX입니다. 특히 <tbody> 요소 내부의 코드입니다.
// before
<tbody>
{issues.map(({ name, message, status }, index) => {
let issueIsOpen = status === "open";
// 우리는 여기서 인덱스 대신 이슈의 id를 사용해야 합니다.
let onClick = issueIsOpen ? () => handleOnChange(index) : null;
let stylesTr = issueIsOpen
? classes.openIssue
: classes.resolvedIssue;
return (
<tr
className={stylesTr}
// backgroundColor는 더이상 checkedState의 일부가 아닙니다.
// 그래서 우리는 이것을 조정해야 합니다.
style={checkedState[index]}
key={index}
onClick={onClick}
>
<td>
{issueIsOpen ? (
<input
className={classes.checkbox}
type={"checkbox"}
id={`custom-checkbox-${index}`}
name={name}
value={name}
// 우리는 여기서 인덱스 대신 이슈의 id를 사용해야 합니다.
checked={checkedState[index].checked}
onChange={() => handleOnChange(index)}
/>
) : (
<input
className={classes.checkbox}
type={"checkbox"}
disabled
/>
)}
</td>
...
</tr>
);
})}
</tbody>
// after
<tbody>
{issues.map(({ id, name, message, status }, index) => {
let issueIsOpen = status === "open";
let onClick = issueIsOpen ? () => handleOnChange(id) : null;
let stylesTr = issueIsOpen
? classes.openIssue
: classes.resolvedIssue;
return (
<tr
key={id}
className={stylesTr}
style={{ backgroundColor: checkedById.has(id) ? "#eee" : "#fff" }}
onClick={onClick}
>
<td>
{issueIsOpen ? (
<input
className={classes.checkbox}
type={"checkbox"}
id={`custom-checkbox-${index}`}
name={name}
value={name}
checked={checkedById.has(id)}
onChange={() => handleOnChange(id)}
/>
) : (
<input
className={classes.checkbox}
type={"checkbox"}
disabled
/>
)}
</td>
...
</tr>
);
})}
</tbody>다시 다음과 같이 요약하겠습니다.
- 우리는 이 코드에서
index의 존재 대부분을 수정해야 합니다. - 이제
<tr>요소에key={index}를 쓰는 대신id를 키로 사용합니다. 이슈의 순서가 바뀌지 않기 때문에 현재 버전에서는 아무것도 변경되지 않습니다. 하지만 이것은 최선의 방법이며 추후에 테이블에 몇 가지 정렬이 추가될 가능성이 높습니다. - 이전에 행의 배경색은
style={checkedState[index]}줄을 통해 설정되었습니다. 이 코드는 이해하는데 시간이 좀 걸립니다. 주어진 행에 대해 이 값은style={{ checked: true, backgroundColor: "#fff" }}로 변환됩니다. - 이와 관련하여 새 버전은 훨씬 더 명확합니다.
style={{ backgroundColor: checkedById.has(id) ? "#eee" : "#fff" }}
여기 Github에서 모든 코드의 변경 사항을 찾을 수 있고, CodeSandbox에서 이 단계의 함수 버전 을 찾을 수 있습니다. (브라우저에서 렌더링된 컴포넌트의 이 버전을 보려면 App.jsx를 변경해야 합니다.)
단계 2: 파생되는 상태 사용
위에서 논의한 두 번째 문제는 불필요한 상태 변수는 selectDeselectAllIsChecked와 numCheckboxesSelected입니다. 언급했듯이, 이것들은 checkedById와 issues 데이터 배열에서 쉽게 파생될 수 있습니다.
// before
function Table({ issues }) {
const [checkedById, setCheckedById] = useState(new Set());
const [selectDeselectAllIsChecked, setSelectDeselectAllIsChecked] =
useState(false);
const [numCheckboxesSelected, setNumCheckboxesSelected] = useState(0);
...
// after
function Table({ issues }) {
const [checkedById, setCheckedById] = useState(new Set());
const openIssues = issues.filter(({ status }) => status === "open");
const numOpenIssues = openIssues.length;
const numCheckedIssues = checkedById.size;
...여기서 openIssuses 변수가 꼭 필요한 것은 아닙니다. 그러나 이렇게 하면 리팩터링된 handleSelectDeselectAll 기능(여기에는 표시되지 않음) 내에서 이 기능을 재사용할 수 있습니다.
이러한 변경 사항 외에도 코드의 나머지 부분에서 상태 설정자에 대한 호출을 제거해야 합니다. 이러한 변경에는 두 가지 이점이 있습니다.
- 우리는 코드 몇줄을 제거합니다.
- 더 중요한 것은 추가 상태를 더 이상 관리할 필요가 없다는 것입니다. 상태를 업데이트 하는 것을 쉽게 잊을 수 있고 이것은 버그를 발생시킬 수 있기 때문에 추가 상태를 제거하는 것은 좋습니다.
마지막 단계로, 우리는 numOpenIssues의 계산을 기억하게 할 수 있습니다. 이는 현재 버전에는 필요하지 않지만 더 큰 데이터 세트를 사용하는 경우 필요할 수 있습니다.
const openIssues = useMemo(
() => issues.filter(({ status }) => status === "open"),
[issues]
);여기 Github에서 모든 코드의 변경 사항을 찾을 수 있고, CodeSandbox에서 이 단계의 함수 버전 을 찾을 수 있습니다.
단계 3: document.getElementById() 제거
리팩터링 단계에서 마지막으로 중요한 단계는 document.getElementById()를 사용하는 대신 상단 체크박스 React 방식으로 접근하는 것입니다.
React의 방식은 무엇일까요? 우리는 ref 사용합니다.
// before
function Table({ issues }) {
...
const handleIndeterminateCheckbox = (total) => {
const indeterminateCheckbox = document.getElementById(
"custom-checkbox-selectDeselectAll"
);
let count = 0;
issues.forEach((element) => {
if (element.status === "open") {
count += 1;
}
});
if (total === 0) {
indeterminateCheckbox.indeterminate = false;
}
if (total > 0 && total < count) {
indeterminateCheckbox.indeterminate = true;
}
if (total === count) {
indeterminateCheckbox.indeterminate = false;
}
};
...
// after
function Table({ issues }) {
...
const topCheckbox = useRef();
const handleIndeterminateCheckbox = (checkedById) => {
const numSelected = checkedById.size;
if (numSelected === 0) {
topCheckbox.current.indeterminate = false;
} else if (numSelected === numOpenIssues) {
topCheckbox.current.indeterminate = false;
} else {
topCheckbox.current.indeterminate = true;
}
};
...
return (
<table className={classes.table}>
<thead>
<tr>
<th>
<input
ref={topCheckbox}
...요약하면 다음과 같습니다.
- 가장 중요한 것은
document.getElementById()를useRef()로 대체했다는 것입니다. 이를 통해 체크박스의id를 제거할 수도 있습니다. - 원래 코드의
issues.forEach((element) => {줄은 해결되지 않은 이슈의 수를 계산하는 역할을 합니다. 이것은 다소 번거로운 접근 방식이며issues.filter(...).length로 쉽게 대체될 수 있었습니다. 그러나 우리는 이미 컴포넌트의 상단의numOpenIssues변수를 정의했습니다. 그래서 코드는 더 이상 필요하지 않습니다. - 우리는 세 개의
if문 대신에if ... else if ... else을 사용할 수 있습니다. 우리는 또한 조건을 전환하여true값 만을 가지게 하였습니다. 이 변경은 제 생각에 우리의 뇌가 쉽게 처리할 수 있도록 해줍니다.
사실, 우리는 심지어 if ... else if ... else이 필요하지 않습니다. 우리는 간단한 코드 한 줄로 대체할 수 있습니다.
topCheckbox.current.indeterminate = numSelected > 0 && numSelected < numOpenIssues;이 줄을 사용하면 handleIndeterminateCheckbox을 유지하는 것도 그다지 의미가 없어집니다.
여기 Github에서 모든 코드의 변경 사항을 찾을 수 있고, CodeSandbox에서 이 단계의 함수 버전 을 찾을 수 있습니다.
마지막 코드
약간의 추가 정리 (변경 내용) 후에 처음 177 LOC(Line of Component)에서 102 LOC로 코드를 줄일 수 있습니다.
import { useMemo, useState, useRef } from "react";
import classes from "./Table.module.css";
function Table({ issues }) {
const topCheckbox = useRef();
const [checkedById, setCheckedById] = useState(new Set());
const openIssues = useMemo(
() => issues.filter(({ status }) => status === "open"),
[issues]
);
const numOpenIssues = openIssues.length;
const numCheckedIssues = checkedById.size;
const handleOnChange = (id) => {
const updatedCheckedById = new Set(checkedById);
if (updatedCheckedById.has(id)) {
updatedCheckedById.delete(id);
} else {
updatedCheckedById.add(id);
}
setCheckedById(updatedCheckedById);
const updatedNumChecked = updatedCheckedById.size;
topCheckbox.current.indeterminate =
updatedNumChecked > 0 && updatedNumChecked < numOpenIssues;
};
const handleSelectDeselectAll = (event) => {
if (event.target.checked) {
const allChecked = new Set(openIssues.map(({ id }) => id));
setCheckedById(allChecked);
} else {
setCheckedById(new Set());
}
};
return (
<table className={classes.table}>
<thead>
<tr>
<th>
<input
type="checkbox"
ref={topCheckbox}
className={classes.checkbox}
checked={numOpenIssues === numCheckedIssues}
onChange={handleSelectDeselectAll}
/>
</th>
<th className={classes.numChecked}>
{numCheckedIssues
? `Selected ${numCheckedIssues}`
: "None selected"}
</th>
</tr>
<tr>
<th />
<th>Name</th>
<th>Message</th>
<th>Status</th>
</tr>
</thead>
<tbody>
{issues.map(({ id, name, message, status }) => {
const isIssueOpen = status === "open";
return (
<tr
key={id}
className={
isIssueOpen ? classes.openIssue : classes.resolvedIssue
}
style={{ backgroundColor: checkedById.has(id) ? "#eee" : "#fff" }}
>
<td>
<input
type="checkbox"
className={classes.checkbox}
checked={checkedById.has(id)}
disabled={!isIssueOpen}
onChange={() => handleOnChange(id)}
/>
</td>
<td>{name}</td>
<td>{message}</td>
<td>
<span
className={
isIssueOpen ? classes.greenCircle : classes.redCircle
}
/>
</td>
</tr>
);
})}
</tbody>
</table>
);
}
export default Table;여기 Github에서 모든 코드의 변경 사항을 찾을 수 있고, CodeSandbox에서 이 단계의 함수 버전 을 찾을 수 있습니다.
제 관점에서 보면, 이것은 더 간결할 뿐만 아니라 훨씬 더 읽기 쉽습니다.
기억해야 할 점
- 다른 상태나 데이터에서 파생될 수 있는 불필요한 상태에 주의하십시오.
- 객체, map 또는 set을 사용하는 것이 배열보다 더 쉽고 효율적일 수 있습니다.
- DOM에 접근하기 위한 리엑트스러운 방법은
document.getElementById()가 아닌useRef()입니다.
hi
code which you have shared it beyond my knowledge area so plz try to explain in detail.
MyBKExperience.com Survey Free Whopper