C#에서 객체지향 남용 리팩토링하기 | CodeMaze

dimohy · 4월 2, 2023, 11:38오후

프로그래밍을 하면서 자주 마주치는 다음의 경우에 대한 리펙토링 글입니다.

스위치 문
임시 필드
상속 거부
인터페이스가 다른 대체 클래스

BigSquare · 4월 4, 2023, 8:09오전

switch 문 같은 경우는 많은 생각을 하게 하는군요.

얼핏 보면, Plane 종류가 늘어날 때마다, Plane에 (기체 타입을 나타내는 필드와 case 추가) 코드를 변경해야 해서, OCP 위반으로 볼 수도 있는데,

그렇다고, 어느 한 시점에, 도메인에서 사용가능한 Plane의 종류를 확정하지 않을 수도 없고.

아래와 같이 Reflection 코드를 사용하면, 뭔가 객체 지향적이 않은 것 같고… 그렇네요.

public abstract class Plane
{
    private static void Main(string[] args)
    {
        var plane = Plane.Create(Plane.Type.Cargo);
        Console.WriteLine(plane.Capacity);
        // 10000

        plane = Plane.Create(Plane.Type.Passenger);
        Console.WriteLine(plane.Capacity);
        // 100

        plane = Plane.Create(Plane.Type.Military);
        Console.WriteLine(plane.Capacity);
        // An unhandled exception of type 'System.InvalidOperationException' 
        // ... .dll: 'Military plane was not defined yet.'
    }


    public abstract double Capacity {get;}

    public static Plane Create(Plane.Type planeType)
    {
        var types = from assembly in AppDomain.CurrentDomain.GetAssemblies()
            from type in assembly.GetExportedTypes()
            where type.IsSubclassOf(typeof(Plane))
            where type.Name.ToString() == planeType.ToString() 
            select type;

        if(types.Any()){
            var type = types.First();
            return (Plane)Activator.CreateInstance(type)!;
        }
        
        throw new InvalidOperationException($"{planeType} plane was not defined yet.");        
    }

    public enum Type 
    {
        Cargo = 0, Passenger = 1, Military = 2,
    }
}

public class Cargo : Plane
{    
    public override double Capacity => 10000;
}
public class Passenger : Plane
{    
    public override double Capacity => 100;
}
// public class Military : Plane
// {    
//     public override double Capacity => 1000000;
// }

dimohy · 4월 4, 2023, 9:32오전

좋은 의견 감사합니다.

개체 생성은 Reflection을 사용하지 않고서는 필연적으로 클래스를 명시할 수 밖에 없어서 저는 OCP 원칙에서 제외 대상이라고 생각은 하고 있습니다.

(물론 Reflection을 사용하지 않고 소스 생성기를 이용하는 방법도 있기는 하겠네요)

suwoo · 4월 5, 2023, 12:43오전

Plane 예제의 경우 보통 Factory 등으로 분리하는 것도 괜찮을 것 같은데,
다른 고수분들 보시기엔 어떨려나요?

날지 못하는 펭귄 예제의 경우에도
strategy를 쓰고 그러는 듯 한데, 이건 너무 많이 나간 것 같네요.

aroooong · 4월 5, 2023, 12:49오전

분기에 따라 상태 관리를 정형화 할 수 있고 많다면,
State Design Pattern 또한 답이 될 수도 있습니다.
어떤 디자인 패턴을 도입 할 것인지에 대해서는 명확한 답은 없다고 생각합니다. ㅎㅎ

다음은 약간 극적인 예시 이지만 State Design Pattern의 찍먹 내용 입니다.

(C#) 미세한 분기(if, else if)리펙토링 - Arooong Blog (arong.info)

suwoo · 4월 5, 2023, 12:53오전

아유, 물론이죠.
명확한 답이 될 수도 없기도 하구요.
좋은 예제 감사합니다. 또 하나 배워가네요 ㅎㅎ

BigSquare · 4월 5, 2023, 1:09오전

Plane은 Factory라는 단어만 안썼지 실제로 팩토리 패턴으로 설계되었다고 봐야하지 않을까요?

제가 걸려했던 부분은 맨 마지막 부분입니다.

An additional advantage is the fact that the new solution is also compliant with the Open/Closed Principle. When adding a new type of aircraft, we no longer have to modify the switch statement, but only add a new subclass.

설명과 다르게, 새로운 Plane 서브 클래스를 만들면, Plane의 switch 문도 수정해야 해서, 여전히 OCP 위배입니다.

예를 들어, Rescue plane이 도입되면, Plane은 이에 대한 case를 추가해야 합니다.

이러한 OCP 위배는 팩토리가 개입된 것이 원인일 수도 있는데,
그렇다고 팩토리를 마냥 없애기에는 뭐 한 것이, 어느 한 시점에 사용 가능한 Plane 종류에 대해 확정해주는 주체가 있을 필요성도 있기 때문입니다.

OCP 를 위배하지 않고 확정하는 방법은 무엇일까하는 고민을 좀 해봤습니다…

suwoo · 4월 5, 2023, 2:15오전

그렇겠네요, 말씀하신 대로 이미 일종의 팩토리네요.
보통은 팩토리를 별개의 클래스로 쓰는 약간 좁은 의미로 생각했던지라 ㅠㅠㅎㅎ
Plane을 자꾸 수정하는 것 보다는 파급력이 작지 않을까 했던 거에요.

마수리 · 4월 5, 2023, 7:23오전

음… 그러네요?

라고 말했지만 글쓴이가 예를 든 리펙토링 방법을 이용해서 분기문에서 구체적인 타입을 반환하던 함수를 추상적 타입을 반환한다고 해서 OCP 위배가 사라지는 것은 아닙니다. 말씀 하신 것처럼 글쓴이의 예제에서 다른 클래스를 추가하면 필연적으로 switch 문에 올바른 객체를 반환하게 추가 해야 합니다. 그렇지 않으면 throw new ArgumentOutOfRangeException($"Incorrect value: {type}") 라고 작업 한대로 Exception이 발생할 겁니다. 어떤 이유에서 OCP를 준수하게 됐다는지 잘 모르겠네용 (설명 해주실분!!)

@BigSquare 님이 말씀해주신 것처럼 도메인에서 어느 시점엔 구체적인 클래스가 필요할 것입니다.
저는 그래서 이럴 땐 코드 흐름상 가장 위에서 추상적 타입을 반환하는 팩토리 클래스를 만들어서 사용하곤합니다. 어쨋든 어느 순간엔 if, switch등을 이용해서 분기를 태워줘야 하는데 흐름상 가장 윗단에서 한번만 분기하는 것이 코드 흐름상 좋아 보이기 때문입니다.

Vincent · 4월 5, 2023, 7:32오전

제 생각에는 OCP를 만족하기 위하여 if-else 나 Switch가 완전히 없어지기만 해야한다는 것은 아니라고 생각합니다.

물론 유연하게 설계되어서 그렇게 할 수 있으면 참 좋겠지만요.

조건문을 박는다.
Enum으로 여러 State를 관리해서 주입받아 사용한다.
조건문을 통으로 Class로 추상화하여 현재 빈번하게 수정되는 현재의 소스코드 파일을 수정하는 대신 추상화된 조건문 클래스 파일 안에서 if-else를 늘려간다.
Dictionary로 사전에 정의된 Enum이나 Class Type의 상태에 대한 동작(Action, Func, Predicate)을 item으로 계속 늘려간다.
동작을 외부 파일로 관리해서 파일을 읽어와서 사용하는 방식으로 한다. (최소한 리빌드는 안하도록)
특정 폴더 안의 모든 클래스를 읽어서 리플렉션 방식으로 처리한다.

등등 당장 머리속에 떠오르는 방식이 저정도입니다.
더 있을 거라고 생각하며, 위에 언급된 내용도 있네요.

제 생각에 가장 OCP스러운 것은 모든 상태와 동작을 class로 추상화 하는 것입니다.
if else를 늘려가는 게 아니라 class 파일이 늘어가는 것이지요.
그리고 받아 쓰는 쪽에 의존성 주입형태로 추상클래스나 인터페이스가 오는 것입니다.

하지만 if else가 코드에 있는 자체는 나쁘다고 보지 않는 편입니다.
개발자는 데드라인과 싸움하며 돈을 버는 회사원이지, 다시 없을 극한의 추상화를 꿈꾸기 보다는 비지니스 로직을 구현하는데에 시간을 부족하게 사용해서는 안된다고 생각하기 때문입니다.
그래서 데드라인까지 마감을 하고 꾸준히 리펙토링을 하려는 마인드셋 역시 테크니컬 적인 부분만큼 중요하다고 생각합니다.

결과적으로 어느 쪽을 선택할 것인지에 대한 문제라고 생각합니다.

BigSquare · 4월 5, 2023, 7:56오전

말씀하신 부분은 원 글의 Plane 예제와 동일한 형태가 아닐까요? 그 글에서도 추상 객체인 Plane을 반환하고 있습니다.
팩토리 기능이 Plane 내부에 있거나, 외부에 있거나, if-else, 혹은 switch 문이 변경되어야 하는 사실은 변함이 없을 것 같습니다.

aroooong · 4월 5, 2023, 8:04오전

저의 생각은

엄밀하게 따지자면 OCP(Open-Closed Principle) 위배가 맞다고 판단되지만,

그것은 일부일뿐 유지보수와 확장성을 높일 수 있는 장점이 더욱 크다고 생각 합니다.

또한 OCP를 지키려면 분기문 형태의 팩토리 패턴이 아닌

실제 특성에 맞는 각각의 팩토리 클래스(서브 클래스)를 정의해서
해당 팩토리 클래스 안에서 인스턴스를 생성해서 제공하는 형태의

추상 팩토리 패턴 디자인으로 설계를 하면 코드의 작업량은 증가할지 몰라도
수정에는 폐쇠적이면서 얼마든지 확장 할 수 있는 OCP 원칙은 지킬 수 있습니다.

BigSquare · 4월 5, 2023, 8:29오전

리플렉션으로 처리할 경우에도, 사용자에게 현재 구현된 서브 클래스의 목록을 전달하는 우아한 수단이 마땅하지 않다는 것이 문제의 핵심 같습니다.

사실 예전부터 동적으로 멤버를 변경할 수 있는 dynamic enum 에 대한 필요성을 느끼곤 했었습니다.

class Rescue : Plane
{
    public Rescue()
   {
        if(Plane.Type.Members.Contains(nameof(Rescue) is false)
           Plane.Type.AddMember(nameof(Rescue));
   }

    public override double Capacity => 2;   
}

마수리 · 4월 5, 2023, 8:32오전

네 맞습니다~ switch문이 변경되어야 하는 사실엔 변함이 없습니다.

@Vincent 님의 의견에도 매우 동의합니다.
다만, 전 개인적으로 분기문을 줄이는 것이 코드 변경 없이 기능을 추가하는 것이라는 생각을 했습니다.

새로운 기능을 추가 할 때마다 여기 if문 1개 추가 저기 if문 한 개 추가 등 너무 구체적으로 많이 아는 로직이 있을 수록 변경할 때마다 같이 변경해줘야 하는 코드들이 많았다는 것을 경험했기 때문입니다.

이러한 이유에서 분기문은 어쩔 수 없이 필요한데 저는 이게 코드 가장 최상단이 되면 좋을 것 같다고 말씀 드린겁니다! ㅎㅎ

Vincent · 4월 5, 2023, 8:38오전

아 Early Return 말씀하신 거였군요.

저도 Early Return은 반드시 사용하는 편입니다 ㅎㅎ

스코프 지옥 싫어요…(아도겐 패턴)

dimohy · 4월 5, 2023, 9:08오전

이런 구조는 어떨까요? 제가 생각하는 선에서는 OCP 원칙을 준수하는 것 같습니다 ^^

// Animal은 Animal.dll로 컴파일 되어 이미 배포되었다고 가정
// 배포된 로직에는 다음의 IAnimalCreator가 추가되어 있다고 가정
Animal.AddCreator(1, new AnimalCreator<Dog>());
Animal.AddCreator(2, new AnimalCreator<Cat>());

// 새롭게 AnimalPig.dll로 Pig가 추가되었다고 가정
Animal.AddCreator(3, new AnimalCreator<Pig>());


// 이후 자유롭게 Animal 개체들을 생성
var dog = Animal.Create(1);
var cat = Animal.Create(2);
var pig = Animal.Create(3);

Console.WriteLine(dog.Name);
Console.WriteLine(cat.Name);
Console.WriteLine(pig.Name);


//---------


/// <summary>
/// Animal은 OCP 원칙을 준수함
/// </summary>
abstract class Animal
{
    private static readonly Dictionary<int, IAnimalCreator> _creatorMap = new();

    public abstract int Type { get; }
    public abstract string Name { get; }

    public static Animal Create(int type)
    {
        var bResult = _creatorMap.TryGetValue(type, out var creator);
        if (bResult is false)
            throw new NotImplementedException($"Type({type}) Animal creator not found.");

        return creator!.Create();
    }

    public static void AddCreator(int type, IAnimalCreator creator) => _creatorMap[type] = creator;
}

/// <summary>
/// 인스턴스 생성자 인터페이스
/// </summary>
interface IAnimalCreator
{
    Animal Create();
}

/// <summary>
/// Animal이 Animal 구현체를 직접 바라보지 않도록 제네릭으로 인터페이스 구현
/// </summary>
/// <typeparam name="TAnimal"></typeparam>
class AnimalCreator<TAnimal> : IAnimalCreator
    where TAnimal : Animal, new()
{
    public Animal Create() => new TAnimal();
}


/// <summary>
/// Animal <- Dog
/// </summary>
class Dog : Animal
{
    public override int Type => 1;

    public override string Name => "Dog";
}

/// <summary>
/// Animal <- Cat
/// </summary>
class Cat : Animal
{
    public override int Type => 2;

    public override string Name => "Cat";
}

/// <summary>
/// Animal <- Pig
/// </summary>
class Pig : Animal
{
    public override int Type => 3;

    public override string Name => "Pig";
}

Vincent · 4월 5, 2023, 9:11오전

코드로 적어주신 게 제가 쓴 글의 4번에 해당한다고 생각합니다.

Dictionary로 사전에 정의된 Enum이나 Class Type의 상태에 대한 동작(Action, Func, Predicate)을 item으로 계속 늘려간다.

따라서 저는 OCP 위반이라고 생각하지 않습니다.
위반인지 아닌지는… 어디까지 위반으로 볼지 기준에 따라 다른거라…

BigSquare · 4월 5, 2023, 9:33오전

원글의 코드보다는 더 좋아 보입니다.
제가 보기에도 OCP에 걸릴만한 것도 없는 것 같습니다.

그런데, 사용자 입장에서, Animal.Create 의 매개 변수로 무엇을 넣으면, 어떤 객체가 생성되는지에 대한 정보는 얻을 수 없는 것 같습니다.

예를 들어, 유효한 매개변수가 1,2,3 만 있는지 더 있는지, 각 번호가 의미하는 객체가 무엇인지에 대한 정보를 얻으려면, .dll을 전부 파헤쳐 봐야 한다면, 유지보수 문제가 있지 않을까요?

이러한 정보는 Enum으로 해결 가능하지만, 이 또한 서브클래스가 추가될 때마다 수정해야 해서 OCP 위반이 된다는 것이죠.

dimohy · 4월 5, 2023, 9:34오전

인터페이스 정적 추상(static abstract) 메서드를 이용해서 좀 더 수정해 보았습니다.

// Animal은 Animal.dll로 컴파일 되어 이미 배포되었다고 가정
// 배포된 로직에는 다음의 IAnimalCreator가 추가되어 있다고 가정
Animal.AddCreator(new AnimalCreator<Dog>());
Animal.AddCreator(new AnimalCreator<Cat>());

// 새롭게 AnimalPig.dll로 Pig가 추가되었다고 가정
Animal.AddCreator(new AnimalCreator<Pig>());


// 이후 자유롭게 Animal 개체들을 생성
var dog = Animal.Create(1);
var cat = Animal.Create(2);
var pig = Animal.Create(3);

Console.WriteLine(dog.Name);
Console.WriteLine(cat.Name);
Console.WriteLine(pig.Name);


//---------


/// <summary>
/// Animal은 OCP 원칙을 준수함
/// </summary>
abstract class Animal
{
    private static readonly Dictionary<int, IAnimalCreator> _creatorMap = new();

    public abstract int Type { get; }
    public abstract string Name { get; }

    public static Animal Create(int type)
    {
        var bResult = _creatorMap.TryGetValue(type, out var creator);
        if (bResult is false)
            throw new NotImplementedException($"Type({type}) Animal creator not found.");

        return creator!.Create();
    }

    public static void AddCreator(IAnimalCreator creator) => _creatorMap[creator.Type] = creator;
}

/// <summary>
/// 인스턴스 생성자 인터페이스
/// </summary>
interface IAnimalCreator
{
    int Type { get; }

    Animal Create();
}

/// <summary>
/// Animal이 Animal 구현체를 직접 바라보지 않도록 제네릭으로 인터페이스 구현
/// </summary>
/// <typeparam name="TAnimal"></typeparam>
class AnimalCreator<TAnimal> : IAnimalCreator
    where TAnimal : Animal, IHaveType, new()
{
    public int Type => TAnimal.ConstType;
    
    public Animal Create() => new TAnimal();
}

interface IHaveType
{
    static abstract int ConstType { get; }
}

/// <summary>
/// Animal <- Dog
/// </summary>
class Dog : Animal, IHaveType
{
    public static int ConstType => 1;

    public override int Type => ConstType;

    public override string Name => "Dog";
}

/// <summary>
/// Animal <- Cat
/// </summary>
class Cat : Animal, IHaveType
{
    public static int ConstType => 2;

    public override int Type => ConstType;

    public override string Name => "Cat";
}

/// <summary>
/// Animal <- Pig
/// </summary>
class Pig : Animal, IHaveType
{
    public static int ConstType => 3;

    public override int Type => ConstType;

    public override string Name => "Pig";
}

dimohy · 4월 5, 2023, 9:42오전

지금은 뾰족하게 떠오르는 생각은 없는데요, DB Provider를 생각해 보면 ConnectionString 처럼 문자열을 이용하는 것으로 보아 방법은 없어 보이는데…

소스 생성기를 굳이 이용하면 가능은 합니다.

더 쉬운 방법은 지금은 떠오르지 않네요 ^^;