숨은 로직 드러내기 | Frontend Fundamentals

[giscus[bot]]

숨은 로직 드러내기 | Frontend Fundamentals

변경하기 쉬운 프론트엔드 코드를 위한 지침서

https://frontend-fundamentals.com/code/examples/hidden-logic.html

[jungsikjeong]

안녕하세요!
사소한 궁금증이 있어서 댓글을 남기게 되었습니다.
다름아니라 아래의 코드에서 반환값으로 Promise<number>이런식의 간단한것은 타입추론 기능으로인해 안적어도 자동으로 리턴값을 추론해주는걸로 알고있습니다.

그래도 반환타입을 명시해주는게 좋을까요?

제가 생각했을때 반환타입을 적어주면 좋은 이점은

• 다른 사람이 내가 작성한 로직을 볼 때 이 함수의 반환타입이 무엇인지 굳이 마우스를 함수이름에올려다보지않아도 바로 눈으로 확인이 가능해서 적어주는게 좋다.

정도인데..
혹시 다른 이유가 더 있는건지 초보 개발자가 조심스레 여쭙니다..

async function fetchBalance(): Promise<number> {
  const balance = await http.get<number>("...");

  return balance;
}

[Choharim]

반환 타입을 제한하는 역할도 한다고 생각합니다!
그래서 리팩터링 등으로 코드가 변경 되었을 때 함수의 역할을 유지하고 사이드 이펙트를 줄일 수 있을 거 같아요

[oungo]

일단 위 코드 구현이 어떻게 되어있는지 알 수 없기 때문에 http.get 의 반환 타입을 알 수 없습니다. 다만 일반적으로 http.get 의 반환타입이 Promise<number>로 타입 추론이 되고 있다고 생각해볼 수 있을 것 같아요. 이 경우에는 fetchBalance 함수의 반환 타입 추론도 잘 될것이기 때문에 굳이 명시하지 않더라도 문제는 없습니다.

하지만 http.get의 반환타입 추론이 제대로 되고 있지 않다면 fetchBalance 함수의 반환타입을 명시해주는게 필요하겠네요. 개인적으로는 위 코드처럼 제네릭을 사용해 http.get의 반환타입 추론이 잘 되도록 하는걸 선호하는 편입니다.

결국 http.get의 반환 타입만 정의하던가 http.get의 반환 타입도 잘 정의하고, fetchBalance함수의 반환타입도 명시해서 한눈에 알아보기 쉽게 하던가 동료들과 협의해서 하나의 통일된 방식을 사용하면 될것같습니다.

[jungsikjeong]

@4kazukin @Choharim @oungo
좋은 지식 담아갈 수 있어 행복하네여
정말 한줄기 햇살 같습니다. 따뜻하네요
다들 답변 너무 감사합니다!

[vreality64]

코드 리뷰 관점에서도 반환 타입을 명시하는게 더 좋긴 해요.
PR 에서는 반환 타입을 알 수 없으니까요. 리뷰어에게 맥락을 더 잘 전달해 줄 수 있습니다.

[jungsikjeong]

코드 리뷰 관점에서도 반환 타입을 명시하는게 더 좋긴 해요.
PR 에서는 반환 타입을 알 수 없으니까요. 리뷰어에게 맥락을 더 잘 전달해 줄 수 있습니다.

오 감사합니다 코드 리뷰 관점에서 접근을 생각못했네요!!

[joseph0926]

이 문서에 대해서 질문이있습니다.
솔직히 변경전 예제와 변경후 예제를 통해서 개선된 부분이 잘 이해가 안갑니다

이전 코드

async function fetchBalance(): Promise<number> {
  const balance = await http.get<number>("...");

  logging.log("balance_fetched");

  return balance;
}

여기서 두가지 문제를 언급하셨는데

1. 로깅이 이루어진다는것을 사용하는 측에서 파악하기 어려움 (의도하지 않은 결과가 나올수있음) => 명시적이지 않음 (암묵적인것보단 명시적인게 낫다)
2. 로깅이 터지면 함수 전체가 터짐 => 역할과 책임이 분리가 안됨

그러면서 아래처럼 개선을 보여주셨는데요,

개선 코드

async function fetchBalance(): Promise<number> {
  const balance = await http.get<number>("...");

  return balance;
}

// 필요한 경우
    const balance = await fetchBalance();
    logging.log("balance_fetched");

    await syncBalance(balance);

개선 코드가 단순히 로깅을 빼고 필요한곳에서 하신다는게 의아합니다.
버튼의 헨들러에서 로깅이 터지는 경우는 어떻게 대비해야하며, 요구사항에 로깅이 필수적이라면 매번 사용하는 곳에서 로깅 코드를 추가해야하는걸까요?

차라리 fetchBalance의 인자로 logging을 받아서 필요할때 logging을 넘겨주는 방식이 더 낫지 않나? 라는 생각이드는데 어떻게 생각하시는지 궁금합니다

[milooy]

좋은 질문 감사해요.

요구사항에 로깅이 필수적이라면 매번 사용하는 곳에서 로깅 코드를 추가해야하는걸까요?

이 부분이 핵심으로 보이는데요, 상황과 요구사항에 따라 다르게 판단해야 합니다 (당연한 말 죄송..ㅎㅎ).

• 예시코드의 함수는 fetchBalance로, 특정 API를 호출하는 단일책임을 갖기를 기대합니다. 만일 logging이 내장되어 있다면 logging API가 설령 없는 노드 환경 등에서 사용하는 케이스에서 불필요한 제약이 생기는게 아쉬워보여요.
• 만일 모든 API 호출시에 로깅이 필요하다면 middleware등으로 해결할 수 있습니다.
• 로깅 함수를 내장하면 말씀주신것처럼 매번 코드를 추가하지 않아도 되어서 편할 수 있지만, log parameter 변경 등 커스텀이 필요해지면 prop을 넘기는 방식으로 확장해야하고, 그러면 fetchBalance 와 직접적으로 관련 없는 prop들이 늘어나게 되는 점이 아쉬워보여요.

어떻게 생각하시는지 궁금해요!

[suinkimme]

저는 이런 숨은 로직 문제를 피하기 위해 로깅을 고차함수로 분리하는 방식도 괜찮다고 생각합니다.
핵심 함수(fetchBalance) 처럼 명시적으로 래핑해서 사용하면 호출부에서 "로깅이 발생한다."는 사실이 드러나서 예측 가능성 문제도 줄어들더라고요.

function withLogging<T extends (...args: any[]) => any>(fn: T) {
  return async (...args: Parameters<T>): Promise<ReturnType<T>> => {
    try {
      const result = await fn(...args);
      logging.log(`${fn.name}_called`);
      return result;
    } catch (error) {
      logToSnapErrors(error);
      throw error;
    }
  };
}

이런 식으로 숨은 로직을 만들지 않으면서도 재사용성과 관심사 분리도 챙길 수 있는 것 같아요.