[알고리즘 || 프로그래머스] 키패드 누르기, 정규식 test() 함수 (feat.비난님의 코드리뷰)

오늘도 할 수 있는 것들을 순서대로 먼저 나열해봤다.

 

function solution(numbers, hand) {
    const click = [];
    const keypad = [
        [1, 2, 3],
        [4, 5, 6],
        [7, 8, 9],
        ['*', 0, '#']
    ]
    const findKey = (n) => {
        let result = [];
        keypad.forEach((line, i) => {
            if (line.findIndex(v => v === n) !== -1) {
                result = [i, line.findIndex(v => v === n)];
                return;
            }
        })
        return result;
    }

    let [leftNow, rightNow] = [[3, 0], [3, 2]];
    numbers.forEach(n => {
        const left = [1, 4, 7];
        const right = [3, 6, 9];
        const leftFn = () => {
            click.push("L");
            leftNow = nPosition;
        }
        const rightFn = () => {
            click.push("R");
            rightNow = nPosition;
        }
        let nPosition = findKey(n);

        if (left.includes(n)) {
            leftFn();
        }
        else if (right.includes(n)) {
            rightFn();
        }
        else {
            let distance = (nowPosition) => Math.abs(nPosition[0] - nowPosition[0]) + Math.abs(nPosition[1] - nowPosition[1])
            const leftD = distance(leftNow);
            const rightD = distance(rightNow);
            if (leftD === rightD) {
                hand === "left" ? leftFn() : rightFn();
            } else {
                leftD < rightD ? leftFn() : rightFn();
            }
        }
    });
    return click.join("")
}

 

left, right 배열에서 findIndex 하는 부분은 정규식으로 바꿀 수 있다.

 

       if (/[147]/.test(x)) {
            leftFn();
        }
      if (/[369]/.test(x)) {
            rightFn();
        }

 

처음에 test 함수가 작성한 사람이 만든 임의의 함수인 줄 알았는데, 

알고보니 정규식 고차함수였다...!!

와!

 

https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test

RegExp.prototype.test() - JavaScript | MDN

 

test() 메서드는 주어진 문자열이 정규 표현식을 만족하는지 판별하고, 그 여부를 true 또는 false로 반환합니다.

만족한다는 건 -> 정규 표현식이 일치하는 부분이 있는가를 의미한다.

 

이렇게 좋은 함수를 이제 알았다니

역시 mdn 은 꼼꼼히 볼 수록 이득이당

 

 

 

 

두구두구 그리고 오랜만에 비난님이 코드 리뷰를 해주셨다.

왜인지 갈 수록 꼼꼼하게 해주시는거 같아서 너무 감사하눼....

고스란히 흡수하면 얼마나 좋을까!

 

 

안녕하세요 또자님 해당 코드 리뷰를 시작하겠습니다.

먼저 기능에 대한 코드를 분석하기 전에 코드 스타일부터 점검하겠습니다.

마침표 종결표 세미클론을 생략 여부를 일관되게 해야합니다.

제출한 코드는  distance 함수만 세미클론을 생략하고 나머지 코드는 세미클론을 붙였습니다. 물론 중간에 생략을 하면 자바스크립트 엔진에서 자동으로 세미클론을 붙여주는 작업을 수행하게 됩니다. 하지만 일관된 방식으로 통일을 해야 코드의 가독성을 높일 수 있습니다.

 

앗... 세미콜론 제대로 붙이는지도 보시다니 비난님은 역시 GPT 아닐까...?

 

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ

이건 걍 웃겨서 가져옴

 

넘나 친절.....

 

 

 

그리고 이제 코드에 대한 기능 부분을 점검하겠습니다.

먼저 기존의 findKey 함수는 forEach를 사용하여 키패드의 각 행을 순회하면서, 
숫자 n이 있는 행을 찾았습니다. 이 때 findIndex 메서드를 사용하여 행 내에서 n의 위치를 찾았고, n이 있으면 n의 위치를 결과에 저장했습니다.

그러나 findIndex를 두번 호출하므로 비효율적일 수 있습니다.

그래서 다음과 같이 findIndex를 한번만 호출하는 것을 추천합니다.

const findPosition = n => keypad.reduce((acc, row, i) => row.includes(n) ? [i, row.indexOf(n)] : acc, []);

리팩토링된 findPosition 함수에서는 reduce 메서드를 사용하여 같은 작업을 수행합니다. 
reduce 메서드는 배열의 각 요소에 대해 주어진 콜백 함수를 실행하면서, 누적값을 결과로 반환합니다. 
이 때 콜백 함수는 현재 요소가 n을 포함하고 있는지 확인하고, n이 있으면 n의 위치를 반환하거나,
그렇지 않으면 누적값을 그대로 반환합니다.

이러한 방식은 findIndex를 두번 호출하는 것을 피하고 includes와 indexOf를 각각 한번 씩 사용하여
호율성을 높였습니다. 그리고 기존의 함수의 이름 key는 객체를 의미할 수 있으며  불분명합니다.

함수가 하는일을 쉽게 이해할 수 있는 이름으로 바꿔야합니다. 
위치를 찾는 함수이기 때문에  리팩토링 된 함수의 이름이 적절합니다.

 

흠 이게 무슨 말일까

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ

 

이해하고 잘못 이해한 줄 데헷

 

이 함수는 keypad의 각 요소 (즉, 각 행)에 대해 콜백 함수를 실행합니다. 
콜백 함수는 두 개의 매개변수를 받습니다. 첫 번째 매개변수는 누적값 (acc)이며, 
초기값은 빈 배열 []입니다. 두 번째 매개변수는 현재 요소 (즉, 현재 행)입니다.

콜백 함수는 현재 행이 n을 포함하는지 확인합니다. 
만약 n을 포함하면, 콜백 함수는 n의 위치 ([i, row.indexOf(n)])를 반환합니다. 
이 위치는 다음 반복에서의 누적값이 됩니다.

만약 현재 행이 n을 포함하지 않으면, 콜백 함수는 누적값을 그대로 반환합니다. 
즉, 누적값은 이전 행에서 찾은 n의 위치를 유지하거나, 
아직 n을 찾지 못했다면 빈 배열 []을 유지합니다.

따라서 reduce 함수는 keypad의 모든 행을 순회하면서 n의 위치를 찾습니다. 
만약 n을 찾으면 그 위치를 반환하고, n을 찾지 못하면 빈 배열 []을 반환합니다.

 

마이 브레인 맞춤형 설명..... 감사하다....

 

 

 

중복코드제거하기

leftFn 과 rightFn
기존의 코드는 leftFn과 rightFn을 따로 정의하였습니다.
해당 함수의 내용은 입력값만 다를뿐 하는일이 같은 함수입니다.
그러므로 해당 함수는 중복을 줄여야합니다.

distance 함수
기존의 코드는 forEach 안에서 조건문를 통해 거리 함수를 정의 하였습니다.

기존의 코드 if 와 else if는 "L" 또는 "R"를 이용하고 위치를 대입합니다.

하지만 distance 함수가 정의된 else 문을 보면 if 와 else...if와 다르게 
추가적인 거리 함수를 정의하며 실행하고 있습니다.

조건문에서 else문만 함수를 정의를 하고 있기때문에

이전에 올린 리팩토링된 함수 (findPostion)와 같은 스코프에 위치하는 것이 적절합니다.

const findPosition = n => keypad.reduce((acc, row, i) => row.includes(n) ? [i, row.indexOf(n)] : acc, []);
const distance = (p1, p2) => Math.abs(p1[0] - p2[0]) + Math.abs(p1[1] - p2[1]);


즉 조건문안에 함수를 정의하지 말고 forEach 문 밖에서 정의하시면 됩니다.

 

오호... 

그러니까 else if 문 안에서 함수를 정의 했는데, ...

이 부분 작성할 때 else 문에서만 쓰니까 else 문 안에 넣어야지~ 하고 쓴건데

 

조건문 전체의 흐름..? 때문에 조건문 밖에서 쓰는 게 적절하다고 이해하면 되는걸까??

 

 

최종 수정된 리팩토링 코드
const solution = (numbers, hand) => {
    const keypad = [[1, 2, 3], [4, 5, 6], [7, 8, 9], ['*', 0, '#']];
    let [left, right] = [[3, 0], [3, 2]];
    const findPosition = n => keypad.reduce((acc, row, i) => row.includes(n) ? [i, row.indexOf(n)] : acc, []);
    const distance = (p1, p2) => Math.abs(p1[0] - p2[0]) + Math.abs(p1[1] - p2[1]);

    return numbers.map(n => {
        const pos = findPosition(n);
        const [leftKeys, rightKeys] = [[1, 4, 7], [3, 6, 9]];

        if (leftKeys.includes(n)) {
            left = pos;
            return 'L';
        } else if (rightKeys.includes(n)) {
            right = pos;
            return 'R';
        } else {
            const [leftDist, rightDist] = [distance(left, pos), distance(right, pos)];
            const result = leftDist < rightDist || (leftDist === rightDist && hand === 'left') ? 'L' : 'R';
            result === 'L' ? left = pos : right = pos;
            return result;
        }
    }).join('');
}

 

구조 분해 할당 문법 사용하기
const left = [1, 4, 7];
const right = [3, 6, 9];

위의 코드는 기존의 코드입니다. left right변수는 서로 연관이 있는 배열입니다. 다음과 같이 바꿀수 있습니다.

 const [leftKeys, rightKeys] = [[1, 4, 7], [3, 6, 9]];

 

3.
기존의 코드는 조건문을 통해 "L"과 "R"을 click 배열에 저장하고  현재 위치를 갱신하였습니다. 
그리고 같은 일을 한다는 것을  알리기 위해 leftFn, rightFn 함수를 정의하는것은 
중복을 줄일 수 있어서 좋은 접근입니다.

 const leftFn = () => {
            click.push("L");
            leftNow = nPosition;
}
const rightFn = () => {
            click.push("R");
            rightNow = nPosition;
 }


다만  해당 함수들은 서로 같은 일을 하고 그리고 함수의 일이 매우 짧습니다. 
그리고 해당 함수를 중복을 묶을 만큼 조건문안에 두가지 일을 하지 않습니다.

그래서 함수를 만들지 않고 조건문을 보다 간결하게 짤수 있습니다. 
특히 else 문을 보시면 두 함수의 내용을 조건문 내부로 직접 이동시켜 중복코드를 제거하였습니다.

if (leftKeys.includes(n)) {
  left = pos;
  return 'L';
} else if (rightKeys.includes(n)) {
  right = pos;
  return 'R';
} else {
  const [leftDist, rightDist] = [distance(left, pos), distance(right, pos)];
  const result = leftDist < rightDist || (leftDist === rightDist && hand === 'left') ? 'L' : 'R';
  result === 'L' ? left = pos : right = pos;

 

4.
그리고 else 문에 있던 복잡한 조건문을 간결시켰습니다.

기존의 코드
else {
  let distance = (nowPosition) => Math.abs(nPosition[0] - nowPosition[0]) + Math.abs(nPosition[1] - nowPosition[1])
  const leftD = distance(leftNow);
  const rightD = distance(rightNow);
  if (leftD === rightD) {
    hand === "left" ? leftFn() : rightFn();
  } else {
    leftD < rightD ? leftFn() : rightFn();
  }
}


리팩토링한 코드
else {
  const [leftDist, rightDist] = [distance(left, pos), distance(right, pos)];
  const result = leftDist < rightDist || (leftDist === rightDist && hand === 'left') ? 'L' : 'R';
  result === 'L' ? left = pos : right = pos;
  return result;
}

 

캬 삼항연산자랑 논리연산자 를 이용해서 저렇게 간단하게 만들 수 있구나...! 

나는 같은 경우랑 다 따로 해는데, 

|| 뒤에 같은 경우도 해당되니까 그 이후에 맞춰서 다시 L 을 출력할지 R 을 출력할지를 결정할 수 있군..!

L, R 에 대해서 먼저 결론을 내리고, 그에 따라 pos (좌표) 를 넣어준다. 

그리고 L 인지 R 인지 return 해버림. 

 

 

마지막으로 forEach 보다 map 함수를 사용하여 각 숫자에 대한 클릭 결과를 즉시 변환을 시켜 
click 이라는 배열을 직접 정의할 필요가 없어졌습니다.

 오... 어차피 배열을 만들거니까 map 을 이용해서 퉤퉤 한 배열 자체를 join("") 한 값을 return 하면 되겠구나...!

 

전체 식에서 return 해야하는 값이 배열이라면 map 이 새로운 배열을 반환한다는 걸 이용하면 몬가 멋진 코드를 짤 수 있군!

 

 

 

 

 

공부 다 했으니 다시 짜보자..!!