Sangmin | Developer

**“함수는 한 가지의 일만 해야한다”**라는 말을 들어 봤을 겁니다.

하지만 “한 가지”라는 것은 보는 관점에 따라 모호할 수 있습니다.

예시를 보며 자세히 살펴봅시다. (올라 앱에 있는 조회하기 버튼을 가상으로 재구성해봤습니다.)

javascript<button onclick="checkEnabledQueryScmInfo(scmData)">
    조회하기
</button>

function checkEnabledQueryScmInfo(scm) {
  if (getInputId(scm.id)|| getInputPw(scm.id)) {
    alert('조회를 하려면 올바른 쇼핑몰 ID 와 비밀번호 입력해주세요.')
    return
  }

	const latestQueryTime = getLatestQueryTime(scm.id)
	const currentTime = new Date()
	const diffTime = diff(currentTime, latestQueryTime)
	if(diffTime > 20) {
		alert(`${diffTime} 이후 다시 시도해주세요.`)
		return
	}
	
	Amplitude.emit(`query-${scm.id}`)
	
  dispatch("queryScm", scm.id)
}

checkEnabledQueryScmInfo은 아래의 역할을 수행하고 있습니다.

버튼 클릭 이벤트 핸들러
인풋 Validation 체크
20초에 한번 실행 되기 위해 시간이 지났는지 체크
데이터 분석을 위한 함수 호출
조회 dispatch 호출

관점에 따라 조회하기 전 체크라는 한 가지의 일을 한다고 말 할 수 있을 것 같습니다.

하지만 “**한 가지의 일을 한다”**라고 말하기 위해서는 <u>코드가</u> <u>변경이 되어야 하는 이유가 하나</u>여야 합니다. 위의 코드는 여러가지 이유로 변경이 되어야 합니다.

코드를 리팩토링하면서 개선해보겠습니다.

인풋 Validation

checkEnabledQueryScmInfo 함수는 input validation 로직에 영향을 받고 있습니다. validation을 책임지는 함수 checkValidScmInput를 만들어 의존을 끊어줍시다.

javascript**function checkValidScmInput(id){
 if (getInputId(id)|| getInputPw(id)) {
      return false
    }
    return true
}**


function checkEnabledQueryScmInfo(scm) {
    **if (!checkValidScmInput(scm.id)) {
      alert('조회를 하려면 올바른 쇼핑몰 ID 와 비밀번호 입력해주세요.')
      return
    }**

		const latestQueryTime = getLatestQueryTime(scm.id)
		const currentTime = new Date()
		const diffTime = diff(currentTime, latestQueryTime)
		if(diffTime >= 20) {
			alert(`최근에 조회된 이력이있습니다. 잠시후 다시 시도해주세요.`)
			return
		}
		
		Amplitude.emit(`query-${scm.id}`)
		
    dispatch("queryScm", scm.id)
  }

코드의 줄은 더 늘어났지만 이제 validation 로직이 변경되어도 checkEnabledQueryScmInfo은 그것에 대해 신경쓸 필요 없어졌습니다.

조회한지 20초 지났는지 체크하는 로직

조회한지 20초가 지났는지 체크하는 로직도 checkEnabledQueryScmInfo 입장에서는 구체적으로 알 필요없습니다. 그러므로 분리해주겠습니다.

javascript
function checkValidScmInput(id){...}

**function checkQueryEnabledTime(id) {
	const latestQueryTime = getLatestQueryTime(id)
		const currentTime = new Date()
		const diffTime = diff(currentTime, latestQueryTime)
		if(diffTime >= 20) {
			return false
		}
}**

function checkEnabledQueryScmInfo(scm) {
    if (!checkValidScmInput(scm.id)) {
      alert('조회를 하려면 올바른 쇼핑몰 ID 와 비밀번호 입력해주세요.')
      return
    }

   **if (!checkQueryEnabledTime(scm.id)) {
			alert(`최근에 조회된 이력이있습니다. 잠시후 다시 시도해주세요.`)
      return
    }**
		
		Amplitude.emit(`query-${scm.id}`)
		
    dispatch("queryScm", scm.id)
  }

Amplitude

코드가 한 줄 뿐 이더라도 다른 책임을 가지기 때문에 분리해주는게 좋을 것 같습니다.

javascriptfunction emitAnialysisScmQueryEvent() {
	Amplitude.emit(`query-${scm.id}`)
}

더 나아가 Amplitude 이벤트를 담당하는 객체를 만드는것도 좋을것 같습니다.

javascriptexport const Ananlysis {
	queryScm(id) { 
		Amplitude.emit(`query-${scm.id}`)
	} 
}

javascriptimport **Analysis** from '@/lib/analyisis'

function checkValidScmInput(id){...}
function checkQueryEnabledTime(id) {...}

function checkEnabledQueryScmInfo(scm) {
  if (!checkValidScmInput(scm.id)) {
    alert('조회를 하려면 올바른 쇼핑몰 ID 와 비밀번호 입력해주세요.')
    return
  }

 if (!checkQueryEnabledTime(scm.id)) {
			alert(`최근에 조회된 이력이있습니다. 잠시후 다시 시도해주세요.`)
    return
  }
	
	**Analysis.queryScm(scm.id)**
	
  dispatch("queryScm", scm.id)
}

버튼 이벤트 분리

코드가 많이 개선 됐습니다. 하지만 아직 부족한 부분이 보이네요. checkEnabledQueryScmInfo만 봐서는 이 함수가 버튼과 연결되어있다고 직관적으로 생각하기 어렵습니다. 버튼 이벤트와 분리 해봅시다.

버튼 클릭시 이벤트를 처리하는 함수 handleQueryClick을 새롭게 만들어 줍니다.

javascriptimport Analysis from '@/lib/analyisis'

<button onclick="handleQueryClick">
    조회하기
</button>

[...]

function handleQueryClick() {
	checkGetScmInfoBefore(scm)
}
function checkEnabledQueryScmInfo(scm) {
  if (!checkValidScmInput(scm.id)) {
    alert('조회를 하려면 올바른 쇼핑몰 ID 와 비밀번호 입력해주세요.')
    return 
  }

 if (!checkQueryEnabledTime(scm.id)) {
			alert(`최근에 조회된 이력이있습니다. 잠시후 다시 시도해주세요.`)
    return
  }
  		
	Ananlysis.queryScm(scm.id)
	
  dispatch("queryScm", scm.id)
}

Query Dispatch 호출 분리

이제 checkGetScmInfoBefore를 좀 더 단순하게 만들어봅시다.

handleQueryClick로 dispatch와 Ananlysis를 옮겨줍니다.

javascriptfunction handleQueryClick() {
	if(!checkGetScmInfoBefore()) return
	Ananlysis.queryScm(scm.id)
  dispatch("queryScm", scm.id)
}

function checkEnabledQueryScmInfo(scm) {
  if (!checkValidScmInput(scm.id)) {
    alert('조회를 하려면 올바른 쇼핑몰 ID 와 비밀번호 입력해주세요.')
    return false
  }

 if (!checkQueryEnabledTime(scm.id)) {
			alert(`최근에 조회된 이력이있습니다. 잠시후 다시 시도해주세요.`)
    return false
  }

}

checkEnabledQueryScmInfo는 비로소 함수의 이름에 맞는 한 가지 역할만 수행하게 되었습니다.

최종 코드

기존의 코드가 기본적으로 심플하기 때문에 크게 개선된것 처럼 느껴지진 않네요.

실제 프로젝트 코드는 훨씬 복잡하기 때문에 함수가 하나의 책임을 갖도록 하는게 중요합니다.

코드는 계속 늘어나고 나쁜 코드는 더 쉽게 퍼집니다. 😢

javascript<button onclick="**handleQueryClick**">
    조회하기
</button>

**function handleQueryClick() {
	if(!checkGetScmInfoBefore()) return
	Ananlysis.queryScm(scm.id)
  dispatch("queryScm", scm.id)
}**

function checkEnabledQueryScmInfo(scm) {
    if (!checkValidScmInput(scm.id)) {
      alert('조회를 하려면 올바른 쇼핑몰 ID 와 비밀번호 입력해주세요.')
      return false
    }

   if (!checkQueryEnabledTime(scm.id)) {
 			alert(`최근에 조회된 이력이있습니다. 잠시후 다시 시도해주세요.`)
      return false
    }
}

function checkValidScmInput(id){
 if (getInputId(id)|| getInputPw(id)) {
      return false
    }
    return true
}

function checkQueryEnabledTime(id) {
	const latestQueryTime = getLatestQueryTime(id)
		const currentTime = new Date()
		const diffTime = diff(currentTime, latestQueryTime)
		if(diffTime >= 20) {
			return false
		}
}

이제 굵은 표시된 코드만 읽으면 전체를 파악할 수 있게되었습니다.

그럼 다음에 뵈요~~