[FE 스터디] Mission 3 코드 리뷰

❗ 변경 전

//main.js
	isRoot: false,
    node: [],
    depth: [],
    selectedFile: [],

 

---

🏆피드백 내용

selectedFile이 결국 사용되는 것은 array가 아닌데 초기 값이 array로 설정되어 있어서 어색한 것 같다.

---

💻변경 후

//main.js

	isRoot: false,
    node: [],
    depth: [],
    selectedFile,

---

❗ 변경 전

  //src/Components/App.js
  //...
  if (prevNodeId === null) {
            //루트 노드로 돌아온 경우
            this.loading.setState({ isLoading: true });
            const rootNode = await fetchAPI.fetchRoot();
            this.loading.setState({ isLoading: false });
            this.setState({
              ...copyState,
              node: rootNode,
              isRoot: true,
            });
          } else {
            this.loading.setState({ isLoading: true });
            const prevNodeData = await fetchAPI.getByDirectoryID(prevNodeId);
            this.loading.setState({ isLoading: false });
            this.setState({
              ...copyState,
              node: prevNodeData,
              isRoot: false,
            });
          }

---

🏆피드백 내용

early return 패턴 사용 ! 
nesting도 줄어들고 가독성에도 도움이 될 것.

---

💻변경 후

  //src/Components/App.js
  //...
  if (prevNodeId === null) {
            //루트 노드로 돌아온 경우
            this.loading.setState({ isLoading: true });
            const rootNode = await fetchAPI.fetchRoot();
            this.loading.setState({ isLoading: false });
            this.setState({
              ...copyState,
              node: rootNode,
              isRoot: true,
            });
            
            return;
          }
          
          this.loading.setState({ isLoading: true });
          const prevNodeData = await fetchAPI.getByDirectoryID(prevNodeId);
          this.loading.setState({ isLoading: false });
          this.setState({
            ...copyState,
            node: prevNodeData,
            isRoot: false,
          });

---

❗ 변경 전

    this.$element.className = "ImageViewer";
    this.$target.appendChild(this.$element);
    this.renderComponent();
    this.onclick = props.onclick;

---

🏆피드백 내용

JS에서는 camelCase를 쓰는것이 더 자연스러워 보일 것 같다.
사소하지만 중요한 점이다.

---

💻변경 후

    this.$element.className = "ImageViewer";
    this.$target.appendChild(this.$element);
    this.renderComponent();
    this.onclick = props.onclick;

 

---

❗ 변경 전

          <div>${selectedFile.name}</div>
        </div>`;
    this.$element.style.display = selectedFile.isVisible ? "block" : "none";
    this.closeImg();

---

🏆피드백 내용

함수명과 실제 하는일을 매치해서 구체적으로 작성해주어야 보기에 편하고 헷갈리지 않는다.

---

💻변경 후

              <div>${selectedFile.name}</div>
        </div>`;
    this.$element.style.display = selectedFile.isVisible ? "block" : "none";
    this.addEventToImageModal();

---

❗ 변경 전

          <div>${selectedFile.name}</div>
        </div>`;
    this.$element.style.display = selectedFile.isVisible ? "block" : "none";
    this.closeImg();

---

🏆피드백 내용

함수명과 실제 하는일을 매치해서 구체적으로 작성해주어야 보기에 편하고 헷갈리지 않는다.

---

💻변경 후

              <div>${selectedFile.name}</div>
        </div>`;
    this.$element.style.display = selectedFile.isVisible ? "block" : "none";
    this.addEventToImageModal();

---

❗ 변경 전

const request = async (url) => {
  const result = await fetch(url);
  switch (result.status) {
    case 200:

---

🏆피드백 내용

좀 중요한 내용입니다.
200만 성공이 아니라 2xx가 성공이라서 그에 맞게 또는 result.ok를 사용하는 것이 좋습니다.

---

💻변경 후

const request = async (url) => {
  const result = await fetch(url);
  if(result.ok){

---