뒤늦게 기능 하나를 추가했더니 내 state가 무너졌다

cansweep·2022년 11월 9일
1
post-thumbnail

지난 포스팅에서 알림 기록 페이지를 구현하며 생긴 트러블 슈팅을 정리했었는데 이 당시 알림 페이지에 들어가는 기능은 오로지 알림이 온 기록을 확인하는 것밖에 없었다.

그러다보니 이 페이지의 용도가 되게 애매하게 느껴졌고 페이지를 좀 더 활용하기 위해 기능을 추가하기로 결정했다.

추가된 기능은 총 두가지이다.

먼저, 알림 기록 페이지에서 메세지마다 해당 알림을 쉽게 수정할 수 있도록 해당 알림 세팅 페이지로 이동할 수 있는 버튼을 추가했다.

그리고 복약 여부를 묻는 텍스트 버튼을 추가했다. 기본 상태는 회색이고 클릭 시 복약 여부를 묻는 모달이 뜬다. 모달 창에서 예를 눌렀다면 해당 알림 기록의 복약 여부는 초록색으로 바뀌고 그다음부터는 복약 여부를 수정할 수 없도록 했다.

구현 & 문제

복약 여부를 추가해야 할 messages state에는 이미 많은 로직이 들어가 있었지만 여기에 복약 여부에 대한 로직도 추가되어야 했다.

interface MessageValues {
  id: string;
  user_id: string;
  user_name: string;
  pill_name: string;
  time: string;
  check: boolean;
  pillBookmarkId: string;
}

먼저 복약 여부를 확인하는 속성을 check라고 이름 짓고 기본값을 false로 설정했다. 그리고 이 값은 사용자가 복약 여부 확인 버튼을 눌렀을 때 true로 바뀌고 고정된다.

const checkMutation = useMutation(() => Api.put(ALARMS.CHECK_ID(id)), {
    onSuccess: () => {
      queryClient.invalidateQueries(messageKeys.getMessages(pageCount));
      onClose();
    },
    onError: () => {
      toast.error(TOASTIFY.FAIL);
    },
  });

사용자가 복약 여부 속성을 변경했을 때 해당 메세지의 check가 true로 업데이트된 알림 리스트를 곧바로 사용자에게 보여주어야 했고 따라서 알림 리스트를 다시 서버에서 가져오기 위해 쿼리를 무효화한 뒤 성공하면 setMessages 로직을 호출했다.

messages state를 업데이트할 때 이전 state와 새로운 state를 합치게 되는데 여기서 문제가 발생했다.

// 알림 목록 중복 제거, 삭제된 알림 목록 필터링
setMessages((prev) =>
  _.uniqBy([...prev, ...alarms], "id").filter(
     (message) => !selectedMessagesId.includes(message.id),
  ),
);

위 코드는 복약 여부 확인 기능 추가 이전의 messages state를 업데이트하는 코드인데 id를 기준으로 알림 목록의 중복을 제거하는 과정을 거치고 있다.

check 속성의 값을 false에서 true로 변경한 이후 messages state를 업데이트할 때 prev에는 check: false가, alarms에는 check: true인 동일한 id의 알림 리스트가 들어있다.

그런데 여기서 id를 기준으로 중복 제거를 하게 되어 이후에 들어온 check: true의 알림 리스트가 제거되는 것이다.

그래서 check: true가 지워지지 않도록 check를 기준으로 정렬하고 이후 중복 제거를 수행했다. 그리고 이를 다시 시간순으로 배치하기 위해 time을 기준으로 한 번 더 정렬하는 과정을 거쳤다.

그랬더니 알아보기도 힘들고 복잡해 보이는 코드가 탄생했다.

/*
 * 1. check 기준으로 sort 후 reverse (check: true가 앞에 위치하도록)
 * 2. id를 기준으로 중복 제거
 * 3. time을 기준으로 sort 후 reverse (최신 알림이 위에 위치하도록)
 * 4. 삭제된 메세지 filter
*/

setMessages((prev) =>
  _.sortBy(
    _.uniqBy(_.sortBy([...prev, ...alarms], "check").reverse(), "id"),
    "time",
  )
  .reverse()
  .filter((message) => !selectedMessagesId.includes(message.id)),
);

동작은 하지만 정말 마음에 들지 않았고 같은 동작을 하는 좀 더 깔끔한 코드로 바꿔야 했다.

해결

제일 마음에 들지 않는 것은 sortBy가 두 번이나 들어간다는 것과 또 원하는 동작을 위해 reverse도 두 번 들어간다는 것이었다. 특히 reverse가 가독성을 해치는데 큰 역할을 한다고 생각했다.

이를 해결하기 위해 lodash의 sortBy 대신 Array의 sort() 메서드를 사용하고 compareFunction 안에서 두 번의 정렬 과정을 한 번에 처리하고자 했다.

Array.prototype.sort() 문서에 따르면 정렬 로직은 다음과 같이 정의된다.

  • compareFunction(a, b)이 0보다 작은 수를 반환 => a가 먼저 온다.
  • compareFunction(a, b)이 0을 반환 => 변경 없음.
  • compareFunction(a, b)이 0보다 큰 수를 반환 => b가 먼저 온다.

그리고 이를 반영해 새로 작성한 코드이다.

/*
 * 1. time 기준으로 역순 정렬(최신 알림 기록이 앞으로 가도록)
 * 2. check 기준으로 역순 정렬(true가 앞으로 가도록)
 * 3. id 기준으로 중복 제거
 * 4. 삭제된 메세지 필터링
*/

setMessages((prev) =>
          _.uniqBy(
            [...prev, ...alarms].sort(function (cur, prev) {
              const prevCheck = Number(prev.check);
              const curCheck = Number(cur.check);
              const prevTime = Date.parse(prev.time);
              const curTime = Date.parse(cur.time);

              if (prevTime > curTime) return 1;
              if (prevTime < curTime) return -1;
              if (prevCheck > curCheck) return 1;
              if (prevCheck < curCheck) return -1;
              return 0;
            }),
            "id",
          ).filter((message) => !selectedMessagesId.includes(message.id)),
        );

알림 기록을 최신순으로 정렬하면서 check: truecheck:false보다 앞설 수 있도록 정렬했고 그 이후 id를 기준으로 중복을 제거해 변경 이전의 알림 기록을 지운 후 삭제된 알림 기록까지 state에서 제거될 수 있도록 했다.

이 과정을 거쳐 사용자는 변경된 알림 기록을 바로 확인할 수 있게 되었다!
팀원분께도 물어보니 확실히 바꾼 뒤가 더 읽기 편하고 이해하기도 쉽다해서 뿌듯했다.

후기

사실 이 코드는 한 달 전에 작성한 코드인데 글로 정리하는 걸 미루고 미뤄 한 달을 채우고 나서야 적을 수 있었다.
정리하면서도 그때 기억이 자꾸 되살아나는데 기능 하나 추가하려다 새벽에 진짜 눈물 나는 줄 알았다. (코드가 진짜 너무 마음에 안 들었다)
앞으로 기능 추가하는 것을 쉽게 생각하지 말 것, 정리는 제때 할 것!을 다짐해야겠다.
그리고 지금도 state 하나에 너무 많은 것이 몰려있는 게 아닌가 싶어 분리해야 할 필요성을 느끼고 있는데 이건 리팩토링하면서 생각해 보기!

profile
하고 싶은 건 다 해보자! 를 달고 사는 프론트엔드 개발자입니다.

0개의 댓글