이 포스팅은 <리팩터링 2판>의 내용을 기반으로 제가 진행중인 프로젝트를 리팩터링한 경험을 바탕으로 작성하였습니다. 보실 때 참고하시기 바랍니다.

긴 매개면수 목록

게임 개발을 하다가 갑자기 악취를 느꼈다.

// Multi_TeamSolider class.....
protected Multi_Projectile UsedWeapon(WeaponType weaponType, GameObject go, Transform weaponPos, 
										Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = 
    		Multi_SpawnManagers.Weapon.Spawn(go).GetComponent<Multi_Projectile>();

    UseWeapon.Shot(weaponPos, dir, speed, hitAction);
    return UseWeapon;
}

이 함수는 투사체를 발사하는 코드이다.
좀 더 구체적으로 설명하면 투사체를 생성 후 Multi_Projectile 스크립트를 가져온 후 내부에 있는 Shot()을 이용해 발사한다.
함수 자체는 짧지만 매개변수를 무려 6개를 받는다..

이렇게 긴 매개변수 목록은 악취를 풍긴다. 칸트 백의 할머니의 교훈에 따라 바로 갈아버리도록 하자.

새로운 함수 제작

매개변수를 지우기 전에 우선 이름이 마음에 안든다. 누가봐도 그냥 투사체를 쏘는 함수인데 Uesd라는 단어를 사용할 필요가 없다.

그럼 우선 마음에 드는 이름의 새 함수를 만들자.

protected Multi_Projectile UsedWeapon(WeaponType weaponType, GameObject go, Transform weaponPos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = 
    		Multi_SpawnManagers.Weapon.Spawn(go).GetComponent<Multi_Projectile>();

    UseWeapon.Shot(weaponPos, dir, speed, hitAction);
    return UseWeapon;
}
    
// 새로운 이름의 함수 생성
protected Multi_Projectile ShotProjectile(WeaponType weaponType, GameObject go, Transform weaponPos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(go).GetComponent<Multi_Projectile>();
    UseWeapon.Shot(weaponPos, dir, speed, hitAction);
    return UseWeapon;
}

테스트 제작

그리고 사전에 하나 더 해야할 일이 있다. 테스팅 제작이다.
지금은 마땅한 테스트 스위트가 없으므로 간단하게 만들도록 하겠다.

// TestUtility class.....

[SerializeField] GameObject arraw;

[ContextMenu("Test UsedWeapon")]
void TestUsedWeapon()
{
    ShotProjectile(WeaponType.Arrow, arraw, transform, transform.forward, 50, null);
}

protected Multi_Projectile ShotProjectile(WeaponType weaponType, GameObject go, Transform weaponPos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(go).GetComponent<Multi_Projectile>();
    UseWeapon.Shot(weaponPos, dir, speed, hitAction);
    return UseWeapon;
}

간단하게 ContextMenu를 이용해서 테스트를 진행하기로 했다.
화살 오브젝트를 자신의 전방으로 발사하는 코드다.

저기 Test UsedWeapon을 클릭하면

오브젝트가 화살을 전방으로 날린다.

저게 뭔 의미가 있나 싶지만 테스트를 자주하게 되면 생각보다 도움이 많이 된다.

함수 선언 변경하기 1 - 타입 변경

테스트도 마련했으니 이제 진짜로 매개변수를 하나씩 살펴보자.
먼저 weaponType. 얘는 안쓰고 있었다.
오브젝트 풀링을 위한 플래그 인수였는데 지난번에 지워놓고 까먹었다.
죽은 코드 제거하기로 고이 보내드리자.

// weaponType 매개변수 삭제
protected Multi_Projectile ShotProjectile(GameObject go, Transform weaponPos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(go).GetComponent<Multi_Projectile>();
    UseWeapon.Shot(weaponPos, dir, speed, hitAction);
    return UseWeapon;
}

이제 진짜로 매개변수들의 역할을 하나씩 살펴보자.

GameObject go는 오브젝트를 스폰하기 위한 변수다.
매개변수로 넘어오는 오브젝트는 Multi_Projectile 스크립트를 가지고 있어야 한다는 전제 조건이 있다.

Transform weaponPos는 투사체의 생성 위치를 가지고 있는 오브젝트다.

저 사진에서 빨간 네모 박스 안에 있는 오브젝트가 있는 곳이 궁수가 공격을 할 때 화살이 소환되는 위치다.

Vector3 dir은 투사체가 날라가는 방향이다.

speed는 투사체가 날라가는 속도다.

hitAction은 맞았을 때 발동되는 액션이다.
대미지를 주는 부분도 있고 유닛 고유의 패시브가 동작하는 부분이기도 하다.
독 대미지를 주거나, 적의 이속을 낮추거나 일정 확률로 골드를 획득하는 패시브 등이 포함되어 있다.

개인적으로 거슬리는 부분이 보이는데 그것부터 수정하고 가자.
우선 go는 무조건 Multi_Projectile를 가지고 있어야 한다. (없으면 null 크래쉬가 남)
그러면 그냥 Multi_Projectile를 받는 편이 직관적일 것이다.

비슷한 맥락으로 weaponPos는 오직 위치로의 역할만 한다. 그러니 Transform으로 받는 것보다 Vector3로 받는 것이 사용하는 입장에서 이해가 좀 더 쉬울 것 같다.

그러면 두 매개변수를 "함수 선언 변경하기" 를 통해 바꾸도록 하자.

[SerializeField] Multi_Projectile arraw;

[ContextMenu("Test UsedWeapon")]
void TestUsedWeapon()
{
    // 바뀐 매개변수에 맞게 함수 사용
    ShotProjectile(arraw, transform.position, transform.forward, 50, null);
}

// GameObject go를 Multi_Projectile projectile로 변경
// Transform weaponPos를 Vector3 weaponPos로 변경
protected Multi_Projectile ShotProjectile(Multi_Projectile projectile, Vector3 weaponPos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon =
        // 스폰 시 projectile의 gameObject를 넘겨준다
        Multi_SpawnManagers.Weapon.Spawn(projectile.gameObject).GetComponent<Multi_Projectile>();
    UseWeapon.Shot(weaponPos, dir, speed, hitAction);
    return UseWeapon;
}

타입 선언을 변경해서 인스팩터가 none이 되었을 테니 다시 적절한 값을 주고 테스트한다.

적절한 값을 할당하고

테스트도 통과했다!

함수 선언 변경하기 2 - 속도 변수 제거

그럼 다음 작업을 하자.

현재 쓰고 있는(그리고 곧 ShotWeapon으로 대체될) UsedWeapon함수를 어떻게 사용하는지 확인해보자.

UsedWeapon(WeaponType.Arrow, arrow, arrowTransform, Get_ShootDirection(2f, target), 50, OnSkileHit);

가장 먼저 눈에 뛰는 것은 속도가 하드코딩되있다는 것이다. 우선 저 부분을 개선하고자 한다.

그 전에 이건 관점에 따라 다르겠지만 속도는 투사체 오브젝트가 가지고 있는 게 좋아보인다.

한 유닛이 여러 개의 오브젝트를 날릴 수도 있는데 그럴 때마다 따로 속도를 나타내는 변수를 추가로 작성하는건 별로라고 생각하기 때문이다.

// Multi_Projectile class.....
public void Shot(Vector3 pos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    OnHit = hitAction;
    photonView.RPC("SetShotData", RpcTarget.All, pos, dir, speed);
    RPC_Utility.Instance.RPC_Active(photonView.ViewID, true);
}

[PunRPC]
public void SetShotData(Vector3 _pos, Vector3 _dir, int speed)
{
    transform.position = _pos;
    Rigidbody.velocity = _dir * speed;
    Quaternion lookDir = Quaternion.LookRotation(_dir);
    transform.rotation = lookDir;
}

이것이 UsedWeapon에서 사용하는 Multi_Projectile 클래스로, 투사체를 정의하는 스크립트다.

새로운 친구 _speed를 작성해주고 기존의 매개변수는 지우자.
하지만 기존의 함수를 건드리면 코드가 깨질 수 있으므로 새로운 함수를 만들도록 하자.

public void Shot(Vector3 pos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    OnHit = hitAction;
    photonView.RPC("SetShotData", RpcTarget.All, pos, dir, speed);
    RPC_Utility.Instance.RPC_Active(photonView.ViewID, true);
}

[PunRPC]
public void SetShotData(Vector3 _pos, Vector3 _dir, int speed)
{
    transform.position = _pos;
    Rigidbody.velocity = _dir * speed;
    Quaternion lookDir = Quaternion.LookRotation(_dir);
    transform.rotation = lookDir;
}


// 새로 작성한 부분
[SerializeField] int _speed;
public void __Shot(Vector3 pos, Vector3 dir, Action<Multi_Enemy> hitAction)
{
    OnHit = hitAction;
    photonView.RPC("__SetShotData", RpcTarget.All, pos, dir);
    RPC_Utility.Instance.RPC_Active(photonView.ViewID, true);
}

[PunRPC]
public void __SetShotData(Vector3 _pos, Vector3 _dir)
{
    transform.position = _pos;
    Rigidbody.velocity = _dir * _speed;
    Quaternion lookDir = Quaternion.LookRotation(_dir);
    transform.rotation = lookDir;
}

그리고 테스트 코드에서 __Shot을 사용하도록 바꾸고, 필요없어진 speed 매개변수를 지우자.

[ContextMenu("Test UsedWeapon")]
void TestUsedWeapon()
{
    // 바뀐 매개변수에 맞춰 값 전달
    ShotProjectile(arraw, transform.position, transform.forward, null);
}

// 쓸모없어진 speed 매개변수 제거
protected Multi_Projectile ShotProjectile(Multi_Projectile projectile, Vector3 weaponPos, Vector3 dir, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(projectile.gameObject).GetComponent<Multi_Projectile>();
    // 새로 만든 __Shot 함수를 사용
    UseWeapon.__Shot(weaponPos, dir, hitAction);
    return UseWeapon;
}

저장하고 테스트하자.

계속 똑같은 화면을 보여주고 있어서 테스트는 안하고 복붙으로 사기치고 있다고 생각할 수 있지만 실제로 테스트하고 있으니 안심해도 된다.

매개변수 객체 만들기

처음 6개였던 반해 매개변수가 4개로 줄었다.
또한 타입도 좀 더 직관적으로 변경했다.
여기서 그만둬도 어느 정도는 성공이라고 할 수 있고 딱 여기까지가 가성비가 괞찮다.

하지만 좀 더 나아지게 할 수 있다.
무었일까? 남은 매개변수들을 살펴보자.
Multi_Projectile projectile : 무기의 원본이다.
Vector3 weaponPos : 무기가 스폰되는 위치다.
Vector3 dir : 발사하는 방향이다.
Action<Multi_Enemy> hitAction : 적에게 히트 시 할 행동이다.

이때 dir과 hitAction은 상황에 따라 변할 수 있다.
dir은 무조건 전방으로 날리는 경우도 있지만 적의 위치에 따라 경로를 계산해야 하는 경우도 있다.
hitAction은 패시브를 강화하는 등 상황에 따라 다른 액션을 실행해야 할 수도 있다.

그렇다면 projectile와 weaponPos는 어떨까?
날리는 화살은 어떤 상황에서도 화살이다. 만약 바꾼다면 무기 자체를 바꾸는 경우일 것이다.
weaponPos도 마찬가지다. 적이 뒤에 있거나 화살에 독뎀을 추가한다 해도 손에서 쏘던 화살을 발가락에서 날리지는 않는다.

즉 projectile과 weaponPos는 한번 정하면 바꿀 일이 없는 불변으로 취급 가능한 데이터다. 비록 2개이기는 하지만 불변 값들은 묶을 가치가 있다.

그러니 "매개변수 객체 만들기"를 통해 2개를 묶는 값 객체를 만들기로 하자.

[Serializable]
public class ProjectileData
{
    [SerializeField] Multi_Projectile original;
    [SerializeField] Transform spawnTransform;
    
	// 생성자
    public ProjectileData(Multi_Projectile original, Transform spawnPos)
    {
        this.original = original;
        this.spawnTransform = spawnPos;
    }
	
    // 게터
    public Multi_Projectile Original => original;
    public Transform SpawnTransform => spawnTransform;
    public Vector3 SpawnPos => spawnTransform.position;
}

이제 만든 클래스를 사용하도록 함수를 바꿔보자

// Multi_Projectile에서 ProjectileData 타입으로 변경
[SerializeField] ProjectileData arrawData;

[ContextMenu("Test UsedWeapon")]
void TestUsedWeapon()
{
    // 바뀐 매개변수에 맞게 함수 사용
    ShotProjectile(arrawData, transform.forward, null);
}

// Multi_Projectile projectile, Vector3 weaponPos 를 ProjectileData data로 변경
protected Multi_Projectile ShotProjectile(ProjectileData data, Vector3 dir, Action<Multi_Enemy> hitAction)
{
    // projectile.gameObject에서 data.Original.gameObject로 변경
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(data.Original.gameObject).GetComponent<Multi_Projectile>();
    // weaponPos에서 data.SpawnPos로 변경
    UseWeapon.__Shot(data.SpawnPos, dir, hitAction);
    return UseWeapon;
}

인스팩터 변수를 바꿨으니 다시 세팅하고 테스트하자.

값을 세팅해주고

테스트도 성공했다!!

자 이제 사용해보자!!

혹시 까먹었을 수도 있지만 지금까지는 테스트 코드에서 함수를 사용하고 있었다.
이제 새로 만든 함수를 실제로 적용해보도록 하자.

그럼 다시 Multi_TeamSoldier 스크립트로 돌아가서 테스트를 하면서 다듬은 ShotProjectile() 함수를 복붙하자.

protected Multi_Projectile UsedWeapon(WeaponType weaponType, GameObject go, Transform weaponPos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(go).GetComponent<Multi_Projectile>();

    Vector3 pos = new Vector3(weaponPos.position.x, 2f, weaponPos.position.z);
    UseWeapon.Shot(pos, dir, speed, hitAction);
    return UseWeapon;
}
    
// 새로운 함수
protected Multi_Projectile ShotProjectile(ProjectileData data, Vector3 dir, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(data.Original.gameObject).GetComponent<Multi_Projectile>();
    UseWeapon.__Shot(data.SpawnPos, dir, hitAction);
    return UseWeapon;
}

사용 중이던 함수를 갑자기 바꾸면 예상치 못한 에러가 나올 수 있으니, 기존의 함수를 새로 만든 함수를 호출하는 전달 함수로 만들겠다.

protected Multi_Projectile UsedWeapon(WeaponType weaponType, GameObject go, Transform weaponPos, Vector3 dir, int speed, Action<Multi_Enemy> hitAction)
{
	// 새로 만든 ShotProjectile() 함수를 사용하도록 변경
	ProjectileData data = new ProjectileData(go.GetComponent<Multi_Projectile>(), weaponPos);
    return ShotProjectile(data, dir, hitAction);
}
    
// 새로운 함수
protected Multi_Projectile ShotProjectile(ProjectileData data, Vector3 dir, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(data.Original.gameObject).GetComponent<Multi_Projectile>();
    UseWeapon.__Shot(data.SpawnPos, dir, hitAction);
    return UseWeapon;
}

마지막 테스트다.
이번에는 테스트 코드가 아니라. UsedWeapon()을 사용하고 있는 실제 게임이 잘 돌아가는지 확인하겠다.

훌륭하게 동작한다.

이제 기존의 UsedWeapon()을 레거시로 명명하고 모든 함수가 ShotProjectile()으로 변경되면 "죽은 코드 삭제하기"로 UsedWeapon()을 지우면 된다.

이 작업은 어느샌가 까먹어버린 __Shot() 함수에도 적용해야 한다.

더 가다듬기

이제 긴 매개변수 목록의 악취는 어느 정도 제거가 되었다.
하지만 아직도 해야 할 일이 남아있다.

잘 보면 투사체를 날리는 함수는 TeamSolider스크립트 안에. 즉 병사들 안에 있다.
지금이야 병사들밖에 투사체를 쓰지 않아 문제가 없지만 이 게임은 디펜스 게임이다.

나중에 타워같은 추가 기능을 구현해야되면 비슷한 코드를 그대로 작성할 확률이 높다.

그리고 애초에 투사체를 날리는 기능이 병사들이 종속적으로 가지고 있는 것은 딱 봐도 이상하다.

즉 "거대한 클래스"가 풍기는 악취다. 이는 "클래스 추출하기"로 닦도록 하자.

클래스 추출하기

우선 새 클래스인 ProjectileShotDelegate 스크립트를 만들도록 하자.

public static class ProjectileShotDelegate
{
    
}

이 클래스는 여러 곳에서 자주 사용할 것 같으므로 static으로 선언했다.

여기에 ShotProjectile()를 복붙하도록 하겠다.

//  public static으로 선언한 부분만 제외하면 똑같다.
public static Multi_Projectile ShotProjectile(ProjectileData data, Vector3 dir, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(data.Original.gameObject).GetComponent<Multi_Projectile>();
    UseWeapon.__Shot(data.SpawnPos, dir, hitAction);
    return UseWeapon;
}

그리고 새로운 버전을 테스트하는 TestUtility를 작성해보자

[SerializeField] ProjectileData arrawData;

[ContextMenu("Shot Test")]
void ShotTest()
{
    ProjectileShotDelegate.ShotProjectile(arrawData, transform.forward, null);
}

잘 된다.

이제 기존의 ShotProjectile() 함수가 새로 만든 함수를 사용하도록 해보자.

protected Multi_Projectile ShotProjectile(ProjectileData data, Vector3 dir, Action<Multi_Enemy> hitAction) 
    => ProjectileShotDelegate.ShotProjectile(data, dir, hitAction);

자 이제 다시 게임 테스트를 해보자.

잘 작동한다!

이제 UsedWeapon()처럼 ShotProjectile()에 레거시를 명명하고 다 바뀌면 "죽은 코드 제거하기" 로 고이 보내드리도록 하자.

방향 계산 함수 옮기기

원래는 이렇게 끝이겠지만 굳이 클래스를 추출한 이유가 있다.
클래스를 따로 추출하는 것의 장점은 연관된 작업들을 추출한 클래스로 가져올 수 있다는 것이다.

지금의 경우 적의 위치에 따라 투사체의 방향을 정하는 함수가 있는데 이 친구가 "매서드 옮기기"를 통해 새로운 클래스로 가져오도록 하자.

// Multi_TeamSolider class.....
Vector3 Get_ShootDirection(Transform attacker, Transform _target)
{
    if (_target != null)
        return (_target.position - attacker.position).normalized;
    else
        return attacker.forward.normalized;
}

딱히 복잡할 것 없는 코드지만 문제는 이게 Multi_TeamSolider 즉 유닛이 직접 계산한다는 게 문제다.

궁수 유닛이 사용하는 함수인데 이를 이용해 ShotProjectile를 사용하면 이렇다.

ProjectileShotDelegate.ShotProjectile(data,
				Get_ShootDirection(transform, target), hitAction)

함수를 사용하는 모습도 마음에 안들고 더 나은 보금자리가 있는데 굳이 옮기지 않을 이유가 없다.

// ProjectileShotDelegate class.....
static Vector3 Get_ShootDirection(Transform attacker, Transform _target)
{
    if (_target != null)
        return (_target.position - attacker.position).normalized;
    else
        return attacker.forward.normalized;
}

옮기기는 했는데 어떻게 사용할지가 고민이다.

나는 함수 오버라이딩을 이용해 새로운 버전의 ShotProjectile() 함수를 만들었다.

// ProjectileShotDelegate class.....

public static Multi_Projectile ShotProjectile(ProjectileData data, Vector3 dir, Action<Multi_Enemy> hitAction)
{
    Multi_Projectile UseWeapon = Multi_SpawnManagers.Weapon.Spawn(data.Original.gameObject).GetComponent<Multi_Projectile>();
    UseWeapon.__Shot(data.SpawnPos, dir, hitAction);
    return UseWeapon;
}

// 새로운 버전의 ShotProjectile() 함수 제작
public static Multi_Projectile ShotProjectile(ProjectileData data, Transform attacker, Transform target, Action<Multi_Enemy> hitAction)
	=> ShotProjectile(data, Get_ShootDirection(attacker, target), hitAction)

static Vector3 Get_ShootDirection(Transform attacker, Transform _target)
{
    if (_target != null)
        return (_target.position - attacker.position).normalized;
    else
        return attacker.forward.normalized;
}

이제 기존의 Get_ShootDirection() 함수 없이 ProjectileShotDelegate의 ShotProjectile() 함수를 사용할 수 있다.

// 새로운 버전의 ShotProjectile() 사용
ProjectileShotDelegate.ShotProjectile(data, transform, target, hitAction)

이제 테스트해보자.

잘 동작한다!! 진짜진짜 끝이다!!

배운 점

음.. 원래는 이런 배운점같은거는 안적으려고 했는데 이번 포스팅을 진행하면서 느낀게 많아서 짤막하게 적도록 하겠습니다.

참고로 갑자기 존대가 된 이유는 리팩터링 과정은 회고록 같은 느낌으로 쓰고 싶어서 말을 놓고 그냥 썻는데 원래 지금까지 포스팅할때는 존댓말로 썻습니다.

아무튼 이번 포스팅에서는 사실 매개변수를 살짝 줄이고 끝내려고 했습니다. 아마 "매개변수 객체 만들기" 쯤부터 계획에 없었던 것 같네요.

사실상 절반 이상 수준의 분량을 추가로 작성한 이유는 그냥 개선할 부분이 "보였기" 때문입니다. 하나 고치고 보니 다른 개선사항이 보이고 또 고치니 또 개선사항이 보이는 상황이 반복되다 보니 이렇게 됐습니다.

또한 포스팅에 포함하지 않은 개선 사항도 많습니다. 정말 <리팩터링 2판>에 나온 것처럼 리팩터링을 진행하면서 저의 코드에 대한 더 깊은 이해를 가지게 되었고 그러다 보니 자연스럽게 개선할 부분이 보인 것 같습니다.

또한 제가 예전에 얼마나 코드를 개판으로 작성했는지도 알게 되었고요. 작성한지 1년 가까이 된 코드도 있어서 도저히 의도를 파악할 수가 없는 내용이 많았습니다.

그나마 다행인건 전과는 다르게 이제는 코드를 개판으로 작성하면 그 코드에서 나오는 악취를 감지할 수 있는 후각이 생겼고, 냄새를 없앨 수 있는 방법이 잘 설명되어 있는 훌륭한 가이드북이 제 책장에 꽃혀있다는 거네요.

혹시 좀 더 나은 코드를 작성하는 것에 관심이 있으시다면 <리팩터링 2판>은 꼭 구매하시기를 권장합니다.