[2022-1-3]프로그래머스 연습 문제 리팩토링

리팩토링 이유

어제 푼 프로그래머스 Level1 - 키패드 누르기 문제에서 코드 속도는 나쁘지 않았지만 가독성이 너무 떨어져서 푸는 도중에 계속 헷갈리고 반복적으로 실수가 나왔다. 지금 내 수준에서 할 수 있는 만큼 리팩토링을 하여 가독성을 최대한 높혀보려 한다.

 

[TIL] - [2022-1-2]알고리즘 문제 풀이, 자바 프로젝트 생성 및 예제 개발 - 어제한 부분

수정 사항

1. Enum

어제 약간 보고 써본 Enum을 조금 더 공부해봤다. Effective Java 아이템34,35 를 참고하고 추가로 검색도 해보면서 개발했다.

1) enum 클래스명 Vertical ->Column

 

키패드가 세로 라인으로 한줄 두줄 세줄 있으니까 이를 나타내려고 Vertical이라는 네이밍을 썻었는데 의미가 잘 안맞는거 같이서 Column 으로 변경하였다. 문제에서 사용한 왼쪽 열, 오른쪽 열, 가운데 열이라는 용어와 Column.LEFT, Column.RIGHT, Column.MIDDLE 과 대응되어 더 의미에 맞게 네이밍 된 것 같다.

 

 

2) enum 바깥에 정의된 getVertical() 메소드를  enum 클래스에서 상수를 반환하는 of() 메소드로 구현

 

이 문제를 풀면서 가장 신경쓰였던 부분이 getVertical 메소드였다. enum 객체와 내부 클래스 Hand에 속하지 못하고 Solution class에서 solution 메서드를 제외한 유일한 메소드 였는데, Enum 상수를 반환하면서 혼자 Solution class에 선언되어 있는 것이 의미가 맞지 않게 느껴졌었다. 

 

이에 대해 enum 클래스에 정적 메소드 of() 를 정의하므로써 해결했다. Column.of(number) 와 같이 사용하고, 키패드 넘버의 열(Column)이라는 의미가 직관적으로 보이기 때문에 가독성을 향상시킬 수 있었다.

 

 

 

3).ordinal() 메소드 사용(X) -> enum 객체 필드에 index 선언 후 getColumnIndex() 메소드를 통해 반환

 

열거타입 상수의 정수값을 얻기 위해 생각없이 ordinal() 메서드를 사용했었는데 이러한 용도로 사용해서는 안된다고 한다. 일단 ordinal()를  통해 반환되는 정수는 열거타입의 상수가 몇번째 위치인지를 반환한다.

때문에 내가 만약 상수를 LEFT(2), MIDDLE(3),... 와 같이 정의하고 Column.LEFT.ordinal()을 통해 2를 반환할기대하지만 실제로 반환되는 값은 0 이다. ordinal은 단순히 열거타입에서의 연속적인 순서를 반환해 주기 때문이다.

만약 상수에 대응되는 정수가 어떤 의미를 갖는다면, 이렇게 연속적인 순서만을 나타내는 ordinal()은 중간에 다른 상수가 추가되거나 하면 순서가 바뀌기 때문에 의미가 바뀌므로 유지보수가 매우 복잡해진다. 

때문에 상수에 연결된 값을 ordinal() 이 아닌 인스턴스 필드에 저장하고, 이를 꺼내는 메소드를 만들어 사용한다. 나는 index를 필드에선언하고 getColumnIndex()를 통해 이를 꺼낼 수 있도록 구현했다.

 

수정 전(가장 간단한 열거 타입과 enum  객체를 반환하는 Solution class의 메서드가 분리되어 있었음)

수정 후

2. Hand 클래스, solution 메소드 정리

solution 메소드 수정 전 / 수정 후 변화

왼쪽의 이미지는 초기의 solution 메소드 내부이다.

별로라고 생각했던 부분을 꼽으면 if 문안에서 비슷한 코드가 반복되고, 문자열에 "L" 또는 "R" 추가하는 코드가 손의 위치를 변경하는 코드와 별개로 실행되는 부분이다. 또한 enum 값을 직접 하드코딩을 넣어주고 있어서 오히려 실수를 내기 쉬운 코드가 되어버렸다. 때문에 Hand 클래스를 수정하여 이와같은 문제를 해결해보고자 했다.

 

1) 변수 answer을 Hand 과 결합

Hand 클래스.

solution method에서 answer.append("L") 와 같이 손 순서를 더해주던 변수 answer을 Hand의 static 변수 handOrder로 선언 해주고 setLocation 메소드가 호출될 때마다 append 해주도록 구현했다. 어떤 손을 append 해줄지는 생성자에서 hand 를 통해 왼손인지 오른손인지를 입력받아 처리하도록 구현했다. 이를 통해 solution method 에서 문자열을 따로 처리해주지 않아도 자동으로 문자열이 처리된다. (final 키워드는 내부 클래스인 Hand 클래스 바깥에서 Hand.handOrder = new StringBuilder(); 와 같이 재할당 해줄 수 없도록 사용했다) 하지만 이와같은 처리방식이 객체지향적인지, 올바른 추상화인지에 대한 의문은 남는다. 아직잘 감이 안오는 것 같다.

 

2) RuntimeException

 

수정 전 / 수정 후

getRowIndex() 메소드를 사용하는 부분

수정 전 코드 getRowIndex 메소드의 마지막에 return -1 을 해주고 있다. 이부분이 로직에서는 일어나지 않는 부분이지만 코드를 잘못 건들이거나 해서 -1을 반환 하면 getRowIndex 가 호출되는 곳에서 Math.abs()를 통해 양수값으로 바뀌기 때문에 기존에 의도한 바와 뜻이 완전 틀려지게 된다. 때문에 -1을 반환하는 것은 절대 일어나서는 안되는 상황이므로 RuntimeException 을 throw하게 개발했다. 

 

실제 개발하면서 Exception 을 throw 해본 것을 스스로 필요하다고 판단해서 해본 것은 이번이 처음이다. 아직 Exception을 공부해보지 제대로 않아서 이렇게 쓰는게 맞는건지 정확하지 않지만 -1을 반환하면 안되는데 어떡하지? 라고 생각하던 차에 이 Exception 을 이용하는 하나의 방법이지 않을까 하고사용해본 것이다. 얼른 공부해보고 내가 맞게 사용한건지 아님 엉터리인지 알고싶다.(근데 stream, enum과 EnumSet,애너테이션, String관련 클래스, 제네릭, 컬렉션,  Spring 도 해야하고, cs도 해야되고 문제도 맨날 풀고 할게 너무 많다ㅠㅠ)

결론 및 느낀점

이밖에도 메소드로 충분히 쉽게 뺄 수 있는 부분인데 문제풀면서 놓치고 비슷한 코드를 여러번 작성하거나, 쓸데없이 특정 메소드를 여러번 호출해서 반환값을 받거나 하는 등의 부분들에서 수정이 이루어졌다. 또한 지금은 아니지만 언젠가 논리적인 오류가 발생할 수 있다고 생각한 부분에 처음으로 Exception 이라는 것을 throw 해보기도 했다. 리팩토링을 한다고 했지만 아직 if문이 세번 중첩되어있는 부분, 문자열을 이와같이 처리하는 것이 맞는 것일지, 네이밍이 적절하게 됐는지, 가독성이 만족할 만한 수준인지 등등에 대해서 아직 많이 부족함을 느꼈다. 공부를 할수록 개발실력에 대한 자신감이 살짝 떨어지는 것 같다. 이 또한 발전의 시작이라고 생각하고 계속 꾸준히 열심히 해야겠다. 언젠가 다시 이 문제를 풀 기회가 생긴다면 훨씬 멋있게 풀 수 있기를..

---

최종 코드

class Solution {

    enum Column{
        LEFT(0),
        MIDDLE(1),
        RIGHT(2);

        private final int index;

        Column(int index){
            this.index = index;
        }

        public int getColumnIndex() {
            return index;
        }

        public static Column of(int number){
            if(number % 3 == 1) return LEFT;
            else if(number % 3 == 2 || number == 0) return MIDDLE;
            else return RIGHT;
        }
    }

    public String solution(int[] numbers, String hand) {

        Hand leftHand = new Hand('*',Column.LEFT,'L');
        Hand rightHand = new Hand('#',Column.RIGHT,'R');

        char charNumber;
        Column columnOfNumber;
        for(int number : numbers){
            charNumber = Character.forDigit(number,10);
            columnOfNumber = Column.of(number);

            if(columnOfNumber == Column.LEFT) {
                leftHand.setLocation(charNumber, columnOfNumber);
            } else if (columnOfNumber == Column.RIGHT) {
                rightHand.setLocation(charNumber, columnOfNumber);
            } else {
                if (leftHand.getDistance(charNumber) > rightHand.getDistance(charNumber)){
                    rightHand.setLocation(charNumber, columnOfNumber);
                }
                else if (leftHand.getDistance(charNumber) < rightHand.getDistance(charNumber)){
                    leftHand.setLocation(charNumber, columnOfNumber);
                }
                else {
                    if (hand.equals("right")) {
                        rightHand.setLocation(charNumber, columnOfNumber);
                    }
                    else {
                        leftHand.setLocation(charNumber, columnOfNumber);
                    }
                }
            }
        }

        return Hand.getHandOrder().toString();
    }

    private static class Hand{

        private static final StringBuilder  handOrder = new StringBuilder();
        private static final char[][] Numbers = {{'1','4','7','*'}, {'2','5','8','0'}, {'3','6','9','#'}};
        private char position;
        private Column column;
        private final char hand;

        private Hand(char initPosition, Column column,char hand) {
            this.position = initPosition;
            this.column = column;
            this.hand = hand;
        }

        private static StringBuilder getHandOrder() {
            return handOrder;
        }

        public void setLocation(char location, Column vertical) {
            this.position = location;
            this.column = vertical;
            handOrder.append(this.hand);
        }

        private int getDistance(char number){
            int distance = Math.abs(getRowIndex(Column.MIDDLE,number) - getRowIndex(this.column,this.position));
            if(this.column != Column.MIDDLE) distance += 1;

            return distance;
        }

        private int getRowIndex(Column column, char position){
            int index = 0;
            for(char number :Numbers[column.getColumnIndex()]){
                if(number == position) break;
                index++;
            }

            if(index == Numbers[column.getColumnIndex()].length) throw new RuntimeException("column 에 position 이 존재하지 않음");

            return index;
        }
    }
}