Skip to content

Latest commit

 

History

History
186 lines (155 loc) · 7.09 KB

혜린 코드리뷰.md

File metadata and controls

186 lines (155 loc) · 7.09 KB

코드 리뷰 및 수정 사항

@김혜린

주요 문제점

  • 들여쓰기 문제점

  • 공백 문제점

  • 문자열 비교 문제점

  • var, let 문제점

  • 의미를 알 수 없도록 네이밍된 변수, 객체화시킬 수 있는 변수들의 남용

    1. 커밋 로그 1
    • typeFlag1, typeFlag2, typeFlag3.. 와 같이 비슷한 기능을 하는 변수들은 따로 배열 또는 따른 객체로 빼서
      작성을 하는것이 좋습니다. 그럴 수 없는 상황이면 각 변수가 어떤 정보를 담는지에 대해 네이밍을 다시 해주세요.
    • console.log() --> 디버그용 코드 모두 지워주세요. (판단하에 불필요 한 것만)
    • 의미 없는 주석은 다 지워주세요.
    case "ROOM_TEMPERATURE" :if(this.state.typeFlag1==="false")
                           {
                            typesTemp.push("ROOM_TEMPERATURE");
                            this.setState({types:typesTemp});
                            this.setState({typeFlag1:"true"})
                          }
                          else
                          {
                           typesTemp=typesTemp.filter(type=>type!=="ROOM_TEMPERATURE");
                           this.setState({types:typesTemp});
                           this.setState({typeFlag1:"false"})
                          }
                           break;
    • 먼저 불린값 비교에 대해서는 "false"보다는 false를 사용해 주세요.
    • 들여쓰기를 수정해 주세요.
    case "ROOM_TEMPERATURE" :
      if(this.state.typeFlag1 === false) {
          typesTemp.push("ROOM_TEMPERATURE");
          this.setState({types:typesTemp});
          this.setState({typeFlag1:"true"})
      } else {
          typesTemp=typesTemp.filter(type => type !== "ROOM_TEMPERATURE");
          this.setState({types:typesTemp});
          this.setState({typeFlag1:"false"})
      }
      break;
    • switch-case문에서 문자열을 비교하는 것은 enum으로 대체해 주세요.
    // 예시
let season = 'summer';

switch(season){
    case 'summer':
        // summer code
    case 'winter':
        // winter Code
    case 'Spring':
        // spring Code
    case 'autumn':
        // autumn Code
}
    • 위 코드의 문제점은 만약 season 변수의 값이 Spring이 아니라 spring일 경우, switch-case문이 정상 작동하지 않습니다.
      이런 경우를 해결하기 위해 enum형을 사용해 주세요. 바꾸면 아래와 같습니다.
    const seasons = {
    SUMMER: 'summer',
    WINTER: 'winter',
    SPRING: 'spring',
    AUTUMN: 'autumn'
}

let season = seasons.SPRING;

switch(season) {
    case seasons.SUMMER:
      // summer code
    case seasons.WINTER:
      // winter code
    case seasons.SPRING:
      // spring code
    case seasons.AUTUMN:
      // autumn code  
}
  1. 커밋 로그 2
  • API문서에서 객체로 제공하는 값들은 일일이 하나의 변수로 따로 두지말고, 객체화 시켜 주세요.
  • 들여쓰기를 수정해 주세요.
// 기존 코드
if(this.state.serviceType==="GENERAL")
{
    axios.post("/warehouses", /* ... */)
}

// 수정된 코드
if(this.state.serviceType === "GENERAL") {
    //...
}

  • 특히 if문과 같은 경우 {를 한 줄 넘겨 쓰지 말고 조건문 뒤에 한 칸 띄어 써주세요.

  • HTML 태그 속성 지정 시 속성마다 한 칸씩 띄어 써주세요.

<!-- 기존 코드 -->
<input type="number" id="depositFee" name="depositFee"onChange={this.inputAll}/><br/>
<label>관리비</label>
<input type="number" id="maintenanceFee" name="maintenanceFee"onChange={this.inputAll}/><br/>
<label>최소사용기간</label>

<!-- 어떤 부분은 띄어쓰기가 되어있고, 어떤 부분은 되어 있지 않습니다. -->
<!-- 아래와 같이 수정해주세요. -->
<input type="number" id="depositFee" name="depositFee" onChange={this.inputAll}/><br/>
<label>관리비</label>
  1. 커밋 로그 3
  • Key: Value의 쌍으로 지정하는 값들에 대해서는 아래와 같이 수정해주세요.
// 기존 코드
monthlyFee:containerDetail.additionalInfo.monthlyFee,
depositFee:containerDetail.additionalInfo.depositFee,

// 수정 코드
monthlyFee: containerDetail.additionalInfo.monthlyFee,
depositFee: containerDetail.additionalInfo.depositFee,
  • :를 기준으로 value에 대해서는 한 칸 띄어 써주세요.
  1. 커밋 로그 4
  • String() 함수보다는 toString()을 사용해 주세요.
    String()은 null, undefined에 대해서도 작동하지만 toString()은 작동하지 않으므로 null과 undefined에
    대한 처리를 더 확실히 할 수 있습니다.
// 기존 코드
{
    String(props.info.serviceType)==="GENERAL"
    ? (<p>월세 : {props.info.monthlyFee}</p>)
    : (<p>종류</p>)
    : (<p>종류 : {props.info.type}</p>)
}

// 수정 코드
{
    props.info.serviceType.toString() === StorageType.GENERAL
        ? (<p>월세 : {props.info.monthlyFee}</p>)
        : (<p>종류</p>)
        : (<p>종류 : {props.info.type}</p>)
}
  • 마찬가지로 문자열 비교는 StorageType.GENERAL과 같이 열거형으로 빼주세요.
  1. 커밋 로그 5

  • itemFlag1, itemFlag2.. 등과 같은 변수는 위와 마찬가지로 배열 또는 객체로 빼거나 그럴 수 없는 경우
    각각 어떤 정보를 담는 변수인지 파악할 수 있도록 네이밍을 다시 해주세요.

  • var과 let의 스코프 범위 차이를 파악하고, 각각 알맞은 상황에 사용할 수 있도록 모두 수정해주세요.

  • ==를 사용한 부분을 ===로 수정해주세요.

  • 피연산자와 연산자 사이에 공백을 추가해주세요.

// 기존 코드
itemsTemp=itemsTemp.filter(item=>item!==e.target.value);

// 수정 코드
itemsTemp = itemsTemp.filter(item => item !== e.target.value);
  • 들여쓰기가 잘못된 부분을 모두 수정해주세요. 특히 switch-case, if-else문이 그런 경우가 많습니다.
  1. 커밋 로그 6
  • src/components/ContainerDetail/ContainerDetail.js에서 typev 변수 네이밍 다시 해주세요.
  • src/pages/Container/Container.js에서 typeFlag1, typeFlag2 와 같은 변수 위에서 말씀드린 대로
    수정해주세요.