취약한 기반 클래스 문제

문제의 발단

레벨 1 마지막 미션인 체스 미션을 진행하다가, 리뷰어에게 이런 피드백을 받은 적이 있습니다.

추상 메서드가 아닌 메서드를 override하는 것은 코드의 예측가능성이나 일관성의 측면에서 위험합니다.

당장은 문제가 없어보이지만, 코드 중복을 이런 식으로 처리하다보면 취약한 기반 클래스 문제를 맞이하게 될 거에요.

이 리뷰를 받게 된 발단은 이렇습니다.

체스 기물 클래스의 최상위 계층인 Piece, 그리고 그것을 상속하는 각각의 구체 기물들이 있는 상황이었죠.

그런데 도메인 로직을 작성하다보니, Piece로 업캐스팅된 구체 클래스가 Pawn인지 확인할 필요가 있었습니다.

다만, 주변의 여러 크루들로부터 “instanceof 연산자는 좋지 않다!” 라는 말을 자주 듣다 보니, 어떻게 해야 instanceof를 사용하지 않고 Pawn인지 체크할 수 있을지 고민을 하게 됐습니다.

이에 대한 결과로, 최상위 계층인 Piece에 isPawn() 이라는 추상 메서드를 만든 뒤, 이 메서드를 각각의 구체 클래스에서 override 하는 방식을 선택하게 되었는데요, 여기서 문득 이런 생각이 들게 됩니다.

“isPawn() 메서드의 결과가 Pawn에서만 true이고 나머지 기물들은 전부 false라면, 굳이 추상 메서드로 만들 필요가 있나? 추상으로 안 만들면 극한으로 코드 중복을 제거할 수 있겠는데?”

꽤나 그럴싸했던 이 생각은 아래와 같은 코드를 만들게 됩니다.

public abstract class Piece {

    public boolean isPawn() {
        return false;
    }
}

public class Pawn extends Piece {
	
    @Override
    public boolean isPawn() {
        return true;
    }
}

내 상속은 취약하다

맨 처음 리뷰어의 피드백 내용이 기억나시나요?

결론부터 말하자면, 추상 메서드가 아닌 메서드를 override 하는 것은 예상치 못한 결과를 초래할 수 있습니다. 즉, 이는 굉장히 위험한 행동이라는 점이죠. 이로 인한 문제점을 취약한 기반 클래스 문제라고 합니다.

그렇다면 이런 행동이 과연 어떤 점에서 위험하다는 것일까요? 간단한 예제를 통해 위험성을 살펴보죠!

조금 특별한 체스

public abstract class Piece {

    private int position = 0;

    public void moveOnce() {
        position++;
    }

    public void moveTwice() {
        position += 2;
    }
    
	// ...
}

public class Pawn extends Piece {

    @Override
    public void moveTwice() {
        moveOnce();
    }

	// ...
}

조금 특별한 체스가 있다고 가정해보겠습니다.

다른 기물들은 앞으로 한 칸 혹은 두 칸을 이동할 수 있는데, Pawn의 경우에는 무조건 한 칸밖에 못 움직이는 룰을 가진 체스죠.

기물을 나타내는 Piece에서는 moveOnce() 그리고 moveTwice() 메서드를 통해 한 칸 혹은 두 칸을 이동하는 기능을 구현하고 있습니다.

다만, Pawn의 경우에는 한 칸밖에 움직일 수 없으니, Pawn 클래스에서는 moveTwice() 메서드를 override 하여 내부 로직을 수정할 필요가 있었습니다.

이를 해결하기 위해, 위 코드처럼 Pawn의 moveTwice() 메서드 내부에 moveOnce()를 호출하도록 override 하게 되었습니다.

이제 Pawn은 무조건 한 칸만 움직일 수 있다는 제약조건을 만족하게 되었네요.

요구사항 변경

시간이 지나 체스 게임의 룰이 변경되었습니다. 기존에 한 칸 혹은 두 칸 움직일 수 있던 룰이, 이제는 무조건 두 칸만 움직일 수 있는 룰로 바뀌게 된 것이죠.

또한, 그동안 새로운 기물들이 많이 추가된 탓에 Pawn 클래스 코드는 점차 잊혀져가는 레거시 코드로 전락하는 중이었습니다.

Piece 클래스 코드를 유지보수하는 개발자들은 바뀐 요구사항을 빠르게 적용하기 위해 고민하다가, 아래와 같은 코드를 고안해냅니다.

public abstract class Piece {

    private int position = 0;

    public void moveOnce() {
        moveTwice();
    }

    public void moveTwice() {
        position += 2;
    }

	// ...
}

moveOnce() 메서드를 아예 사용하지 않게끔 하는 것이 제일 좋겠지만, 워낙 코드량이 방대한 탓에 moveOnce() 메서드가 오용되고 있을 가능성이 존재했습니다.

그래서 그냥 moveOnce() 메서드가 호출되어도 내부적으로는 moveTwice() 메서드를 호출하도록 하여, 두 칸 이동과 같은 효과를 내도록 수정하게 된 것이었죠.

치명적인 문제점

Piece 코드를 유지보수하는 개발자들은 당연히 저 코드가 잘 동작할 것이라 생각했습니다. 겉보기에도 별 문제가 없어보이니까요.

더군다나, moveOnce() 와 moveTwice() 는 추상 메서드도 아닙니다. 이는 각 구현체들이 알아서 구현하지 않아도 되는, 즉 모든 기물들이 “일반적”으로 사용하는 메서드 라는 의미를 내포하고 있죠.

이 시점에서 Piece 코드와 Pawn 코드, 그리고 기물을 움직일 때 사용되는 ChessGame 코드를 같이 봐볼까요?

public abstract class Piece {

    private int position = 0;

    public void moveOnce() {
        moveTwice();
    }

    public void moveTwice() {
        position += 2;
    }

	// ...
}

public class Pawn extends Piece {

    @Override
    public void moveTwice() {
        moveOnce();
    }

	// ...
}

public class ChessGame {

	// ...

    public void movePiece(Piece piece) {
        piece.moveTwice();
    }

	// ...
}

만약 ChessGame의 movePiece() 메서드 파라미터로 Pawn이 오게된다면 어떤 결과가 나올까요?

재귀를 무한으로 즐기게 되면서 스택 오버플로우가 발생하게 됩니다.

이러한 결과가 과연 개발자가 예상하고 기대한 결과일까요? 결코 아니겠죠.

이런 문제점을 취약한 기반 클래스로부터 오는 문제라고 말합니다.

어쩌다가 나의 상속은 취약해졌을까

문제의 시작점은 과연 어디일까요? 대체 언제부터 취약한 구조를 가지게 됐을까요?

바로 추상 메서드가 아닌 moveTwice() 메서드를 하위 클래스에서 override 했을 때부터입니다. 어쩌면 상속 구조를 사용하기로 결정했을 때부터일 수도 있구요.

취약해진 Piece 코드를 유지보수하는 개발자의 입장에서 생각해볼까요?

요구사항 변경으로 인해 Piece 내부의 메서드 로직 수정이 필요할 때, 개발자는 Piece 하위의 클래스들이 해당 메서드를 어떻게 override 하고 있는지 반드시 신경써야합니다.

어떤 하위 클래스에서는 override가 안되어있고, 어떤 하위 클래스에서는 override가 되어있는 굉장히 애매한 상황때문에, moveOnce() 메서드 하나만 변경하고 싶어도 많은 하위 클래스들의 코드를 뜯어봐야하는 상황인거죠.

즉, 캡슐화가 전혀 지켜지지 못하고 있는 상황입니다. 구현에 대한 상위와 하위 클래스 간의 결합도가 굉장히 높으니까요.

정리해보자면, 일관성 없는 무지성 override가 상속 구조를 굉장히 취약하게 만들고, 유지보수하는 개발자가 신경써야하는 포인트를 늘리고 있습니다.

반드시 신경써야하는 포인트가 늘어날수록, 코드의 흐름 예측이 더 어려워져 의도치 않는 결과를 불러오게 됩니다. 결국 유지보수는 사람이 하는 일이고, 관리 포인트가 늘어나면 실수가 생길 확률이 올라가니까요.