Code Review # 1

Before

function toggleSelectedQS(key, value) {
	if (searchParams.toString().includes(`${key}=${value}`)){
		[...searchParmas.entries()].filter(([newKey, newValue]) => {
		if (key !== newKey) return true;
		else if (value !== newValue) return true;
		else return false;
		})
		setSearchParams(new URLSearchParams(searchParams))
	}
	else {
		const newSearchParams = new URLSearchParams(searchParams);
		newSearchParams.filter()
	}
}

After

function applyNextQueryString(next) {
	setSearchParams(next);
}
function toggleSelectedQS(selectedQueryKey, selectedQueryValue) {
    let nextParamsEntries = [...searchParams.entries()];
    const isFilterExist = checkEntryExist(
      selectedQueryKey,
      selectedQueryValue
    );

    // selectedQueryValue가 QS에 있다면 제거
      const newSelectedEntry = [selectedQueryKey, selectedQueryValue];
    const nextEntries = [...searchParams.entries()];

    return isValueExist
      ? [...nextEntries, newSelectedEntry]
      : nextEntries.filter(filterOutExistValue);

    function filterOutExistValue([queryKey, queryValue]) {
      return queryKey !== selectedQueryKey && queryValue !== selectedQueryValue;
    }

고칠점

우연찮게 라이브로 코드리뷰를 해주시면서 리펙토링 하시는 걸 지켜보았다. 보며 느낀점은 크게 2가지!

1) 가독성과 네이밍

• Before의 코드를 보면 조건들이 굉장히 많고 복잡하다. 추후에 코드를 읽는 경우 key, value라고만 떡하니 써 있는 경우에는 무엇에 대한 변수이고 하는일이 무엇인지에 대해 보는 과정이 너무 어려워진다. 조금은 길어지더라도 정확한 네이밍이 필요!! 단, 너무 길어도 문제!! 모두가 알만한 줄임말 외에는 피하도록 하고 최대 3단어만으로 끝내자!

2) 함수명과 기능

• (1)의 네이밍과 이어지는 부분이다. 추가로 함수를 기능적으로 나누는 것!함수가 어떤 것을 하는 것인지, 한가지 기능만을 하도록 관심사를 충분히 나눌것!
• 가장 충격이었던 점은 setSearchparams( )라는 단 1개의 기능만 하는 것을 함수로 따로 빼신 것! 처음에는 딱 1줄의 로직을 왜 따로 함수로 만드는 지 의문이었지만 계속해서 사용해본 결과 setSearchParams로 쓰는 것보다 훨씬 명시적이었다.