Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[태영] 담님께 드리는 제 미천한 코드리뷰 #1

Closed
taeyoungs opened this issue May 24, 2023 · 0 comments
Closed

[태영] 담님께 드리는 제 미천한 코드리뷰 #1

taeyoungs opened this issue May 24, 2023 · 0 comments

Comments

@taeyoungs
Copy link

taeyoungs commented May 24, 2023

안녕하세요. 담님!

빡준님께서 체크하시고 피드백 반영한 부분을 제외한 나머지 부분에서 제 개인적인 취향이 들어간 코드 리뷰를 남겨봅니다.
가볍게 봐주세요. 저는 개발먼지니까요!
그럼 바로 시작하겠습니다!

ApiUrl

굳이 클래스일 필요가 있을까? 라는 생각이 들긴 해요. 대문자 변수명을 이용한 상수 처리 정도면 충분하지 않았을까 .. 라는 생각이 있습니다.

const API_URL = {
  BASE_URL: "https://marvel-proxy.nomadcoders.workers.dev/v1/public/characters",
  LIST: `${API_URL.BASE_URL}?limit=50&orderBy=modified&series=24229,1058,2023`;
}

여담이고 이미 알고 계실 수도 있지만 클래스라는게 단순히 자바스크립트에서의 또 다른 객체 생성 방식이라고 할 수도 있지만 실제로 MDN 문서 상단에 적혀 있기도 하고

Class는 객체를 생성하기 위한 템플릿입니다.

근데 보통 객체지향 프로그래밍 언어에서의 클래스는 객체지향 설계를 위한 도구에요. 객체지향 설계를 통해 객체들간의 협력 관계를 구상해놨는데 각 객체들이 협력을 하려면 객체들이 자신만의 상태를 가지고 있어야 하고 협력을 요청할 수도 있어야 하며 요청을 받아서 처리를 할 수도 있어야 한단 말이죠.

그러한 각 객체들의 상태, 역할, 책임을 구현하기 위해 클래스라는 도구를 사용해서 표현하는 거죠.

예를 들어, 롤러코스터 타이쿤이라는 놀이공원 운영 게임을 만든다고 했을 때 놀이공원이니 사람이 필요할 거고 놀이공원 내에는 사람이면서 손님이 있을거고 상점 주인이 있을 것이며 청소하는 사람도 있을 수 있겠죠.

타이쿤

이런 사람들이 가만히 있느냐? 그렇지 않죠. 저희가 놀이공원을 갔을 때 츄러스를 사먹듯이 손님 역할을 맡은 객체가 상점 주인 역할을 맡은 객체에게 특정 요청을 하고 그 요청을 받아 상점 주인 객체는 처리를 할 수도 있어야 하죠.

이런 말을 설명을 주저리 주저리 하는 이유는 왠지 클래스를 한번 써보고 싶어서 쓴 느낌을 받아서에요. 제가 예전에 그랬거든요. 그게 아니라면 다행입니다! (넘겨짚기 장인 ..)

사실 프론트엔드 개발자 특히나 리액트 개발자가 클래스를 깊게 다룰 일이 많지는 않다고 생각해요. 아무래도 함수 컴포넌트를 택한 이후로 더 클래스를 만날 일이 줄었다고 생각이 듭니다. 그렇다고 해서 클래스를 몰라도 되냐는 아니라고 개인적으로 생각해요.

개발자인 이상 객체지향 설계, 언어에 대한 이해는 필요하고 특히나 라이브러리나 프레임워크를 만들거나 이해하기 위해선 객체지향이 더욱 더 필수적이라고 생각합니다. 예를 들어, 리액트 없이 바닐라 자바스크립트로 리액트와 비슷하게 만들려고 하면 상당히 높은 확률로 클래스와 객체지향 설계에 대한 이해가 필요해요(이건 혹시나 나중에 시간이 되신다면 해보세요! 재밌습니다).

말이 길어졌네요. 여기서 마무리하고 객체지향이 조금 궁금해진다 하면 객체지향의 사실과 오해라는 책을 추천드릴게요!

api.ts

fetcher를 분리하신 건 정말 좋네요! 👍🏼

useGetDetail, useGetList

관심사를 분리해주신 것도 너무 좋네요! 빡준님께서 여러 분께 공통적인 피드백을 드리는 걸로 알고 있는데 관심사를 분리한다는 말 자체가 어려울 수도 있다고 생각은 합니다.

저라고 잘 하는건 아니지만 저도 평소에 최대한 관심사를 분리하려고 노력합니다.

리액트에는 훅이라는 좋은 도구가 있기 때문에 특정 작업에 관련된 코드만 모아서 훅에다가 담아주면 해당 훅을 사용하는 곳에선 특정 작업에 대한 상세 내용은 몰라도 되고 훅에서 반환된 값만 받아서 사용하면 되는 그림이 나오게 됩니다.

예를 들어, Detail이라는 페이지 컴포넌트를 구현하고 있을 때 해당 컴포넌트에서 API URL이나 이펙트 함수 내부 로직이 페이지 컴포넌트 입장에서 궁금한 부분일까요?

UI를 표현하기 위한 컴포넌트는 UI를 표현하기 위한 데이터만을 받아서 뿌려주는 걸로 역할이 충분할 수 있지 않을까요?

사실 이건 컴포넌트가 복잡해지기 시작하면 간단한 부분은 아닌데 이런 맥락이다 라고만 생각하시면 좋습니다. :)

그리고 지금 이 두 개의 훅을 보고 더 나아갈 수도 있을 것 같아요. 두 훅의 모양새가 꽤나 비슷한 상황입니다. 두 개의 훅에서 공통적인 부분을 뽑아내 추상화 해볼 수도 있을 것 같아요!

useFetch 같은 공용 훅이 생기는거죠! 그러면 매번 훅을 만들 때 로딩 상태를 만들어 주고 이펙트 함수를 만들어줄 필요가 없게 될 수도 있겠네요! useGetDetail에서는 useFetch를 사용하되 Detail 페이지에서 필요한 특정 값을 변환하거나 계산하는 작업을 할 수도 있겠구요.

그런게 딱히 없다면 바로 useFetch 훅을 useGetDetail이라는 훅을 만들지 않고 사용해도 되구요.

이런 공용 훅이 관심이 생기신다면 타입스크립트를 이미 사용하고 계시니 usehooks-ts를 추천해드릴게요!

types

이건 소소하긴 한데 types 폴더 내에서 types > types.ts로 가기 보다는 character.ts로 가준다거나 공용 타입이 있다면 common.ts로 나눠줘도 좋을 것 같아요!

생각보다 길어졌네요! 개발 먼지는 이만 퇴장하겠습니다. 🫠

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants