부적절한 코드

 

 

 

 

• 공용 함수로 빼려는 노력이 보이지만, 실제 로직은 특정 기능에 대해서 한정되어 있다.

• 범용성을 띄도록 수정하려면 sort() 함수를 인자로 전달받도록 해야하는데, 그러면 js의 sort를 사용하는 것과 같다.

• 따라서 특정 기능에 대한 기능으로 유지하고, 위치만 해당 기능에 맞는 폴더로 이동시켜 주는게 좋다고 판단했다.

 

• 하나의 상태값에 따라 if else로 나뉘는 케이스인데, 각각을 따로 조건문 검사를 하도록 되어 있다.

• 사실상 하나의 상태에 대한 처리이므로 이 의도가 드러나도록 코드로도 작성되어야 한다.

• 따로 있다면 두 조건문을 묶어서 보지 못하며, 컴퓨팅도 불필요하게 조건검사가 한번 더 이루어진다. (크진 않지만)

 

• 의존할 필요가 없는 의존성을 추가하여 불필요한 예외처리

• 단순히 타입에러를 없애기 위한 보장되지 않은 타입 어설션 사용

 

• 공지모달이 토출가능여부에 대해 의존성을 가질 이유가 없다.

• 불필요한, 부적절한 의존성은 제거 되어야 한다.

 

• (인자) ⇒ ( ()⇒void ) 의 형태로 인자를 주입받는 것이 부자연스러움.

 

• onPress 프롭 자리에 onPress 함수를 호출하는 형태로 전달하는 것이 부자연스러움

 

• 컴포넌트를 합성으로 받고자 ReactElement 타입으로 선언되어 있는데, 컴포넌트 반환 함수를 만들어서 넣어주고 있었다.

• 컴포넌트 코드 자체가 함수이므로 그냥 넣으면 된다. 불필요한 구문이므로 삭제 건의

 

• 고정된 텍스트를 보여주기 위함인데, Input 컴포넌트를 사용하고 있다.

• 값의 입력을 원하는 게 아니므로, 수정이 안되도록 editable=false를 주고 있다.

• Input 대신 Typography 컴포넌트로 단순히 텍스트를 렌더링 시키는 것이 더 적절하다.

 

• 링크를 생성하는 코드이고, 실패할 가능성이 있는 코드다.

• 반환값은 string이며, 링크가 생성되기 전에도 기본값이 ‘’ 으로 string 타입에 부합된다.

• 따라서 링크가 생성되지 못했더라도 반환타입에 부합하므로, 사용하는 쪽에서는 링크가 있다고 생각하고 처리하기 쉽다.
• 예외 처리를 하려면 !== ‘’ 으로 처리해야하는데, 반드시 내부 코드를 봐야만 알 수 있는 정보다.

• 초기값을 null로 두고, 링크 생성에 실패했을 땐 null이 반환되도록 하는 것이 휴먼에러도 줄이고 훨씬 와닿는 코드가 된다.

 

• 불필요하게 전체 데이터 2번 순회

• 전체 데이터 순회를 하더라도 로직이 명확하게 구분되는 케이스라면 안고가도 되지만, 연관되는 로직이므로 두번 도는 것이 의미가 없다.

 

• CartridgeReader, CartridgeData, CartridgeWriter 이렇게 세 가지 객체가 따로 존재한다.

• Data를 Reader가 내장하는 방식도 논의되었지만 워낙 글로벌하게 자주 참조되는 영역이라서 Data를 공용 데이터로 분리해서 사용하기로 합의하였다.

• 그런데 Reader 에서 Data를 제공하도록 메소드 단 하나만 이렇게 수정되는 방향은 옳지 않다.

• 만약 CartridgeReader 안에서 Data의 내용이 제공되어야 한다면, 아예 Data를 내장시키고 직접 참조할 수 없는 형태로 클래스 구조 자체가 변경되어야 옳다.

 

• 로직이 잘못된 코드는 아니고, 가독성과 코드이해 관점에서 수정 포인트다.

• filter의 목적은 장착된 보틀만 필터링 하겠다는 의미인데, cartridge?.firmwareNutrientName 까지 접근한 후에 값이 있는지를 보는 방식으로 필터링 하기 보다, cartridge : CartridgeInfo | null 이므로 filter((cartridgeInfo) ⇒ cartridgeInfo)로 있는 보틀들로 추리는 과정이 선행되는 것이 더 직관적이고 이해하기 쉽다.

• 단순히 순서를 바꿔주는 정도의 리팩토링임에도 효과는 생각보다 크다.