Как провести рефакторинг абстрактного класса, который имеет открытые члены / свойства для каждого конкретного типа? - PullRequest
Новая
       35

Как провести рефакторинг абстрактного класса, который имеет открытые члены / свойства для каждого конкретного типа?

1 голос
/ 17 апреля 2019

Я смотрю на унаследованный код и наткнулся на абстракцию, которая имеет свойства для каждого из его производных / конкретных типов.Я не могу поделиться точным кодом, но, пожалуйста, представьте, что вместо простой операции существуют многочисленные более сложные операции.

Я не встречал ничего подобного раньше и у меня много вопросов?Во-первых, это шаблон, который я не знаю?Если так, то, что это?Второй вопрос, как мне провести рефакторинг этого, чтобы он следовал твердым принципам?

Я постараюсь сделать лучший пример, если это необходимо. 

public enum ToolType
{
    Unknown = 0,
    HRMonitor,
    Dumbell,
    SomeForceDevice
}

public abstract class ToolData
{
    private ToolData()
    {
        IsValid = false;
        this.ToolType = ToolType.Unknown;
    }

    public ToolData(ToolType toolType)
    {
        this.ToolType = toolType;
    }

    public ToolType ToolType { get; }
    public virtual bool IsValid { get; protected set; } = true;
    public double LinkQuality { get; set; }


    public NullToolDataValue NullData => this as NullToolDataValue;
    public DumbellDataValue DumbellData => this as DumbellDataValue;
    public HeartRateDataValue HRData => this as HeartRateDataValue;
    public SomeForceDataValue SomeForceData => this as SomeForceDataValue;
}

public class NullToolDataValue : ToolData
{
    public NullToolDataValue() : base(ToolType.Unknown)
    {
        IsValid = false;
    }
}

public class DumbellDataValue : ToolData
{
    public double WeightValue { get; private set; }

    public DumbellDataValue(double weightValue) : base(ToolType.Dumbell)
    {
        this.WeightValue = weightValue;
    }

    public override string ToString()
    {

        return WeightValue.ToString(CultureInfo.InvariantCulture);
    }
}

public class HeartRateDataValue : ToolData
{
    public int HeartRate { get; private set; }

    public HeartRateDataValue(int heartRate) : base(ToolType.HRMonitor)
    {
        this.HeartRate = heartRate;
    }

    public override string ToString()
    {
        return HeartRate.ToString(CultureInfo.InvariantCulture);
    }
}

public class SomeForceDataValue : ToolData
{
    public double LeftHandForceValue { get; private set; }
    public double RightHandForceValue { get; private set; }

    public int LeftHandPosition { get; private set; }
    public int RightHandPosition { get; private set; }

    public SomeForceDataValue(double lefthandValue, double rightHandValue, int leftHandPosition, int rightHandPosition) : base(ToolType.SomeForceDevice)
    {
        this.LeftHandForceValue = lefthandValue;
        this.LeftHandPosition = leftHandPosition;
        this.RightHandForceValue = rightHandValue;
        this.RightHandPosition = rightHandPosition;
    }

    public override string ToString()
    {
        return $"{LeftHandForceValue.ToString(CultureInfo.InvariantCulture)}" +
            $"| {LeftHandPosition.ToString(CultureInfo.InvariantCulture)}" +
            $"| {RightHandForceValue.ToString(CultureInfo.InvariantCulture)}" +
            $"| {RightHandPosition.ToString(CultureInfo.InvariantCulture)}";                
    }
}

Он используется /потребляется через что-то вроде ниже, что тоже не хватает наследования и вещей для краткости: 

public class DumbellExcercise
{
    public void ToolDataReceived(ToolData data)
    {
        if (data?.DumbellData == null) return;

        //add value to some collection 
        Collection.Add(data.DumbellData.WeightValue);
    }

}

public class HRExcercise
{
    public void ToolDataReceived(ToolData data)
    {
        if (data?.HRData == null) return;

        //add value to some collection 
        Collection.Add(data.HRData.HeartRate);
    }

}

Ответы [ 2 ]

0 голосов
/ 18 апреля 2019

Увидев ваши правки, я еще более твердо убежден, что способ реорганизовать этот код - использовать сопоставление с образцом. Для сопоставления с образцом требуется по крайней мере C # 7.0, поэтому я включу почти такой же хороший способ сделать это до версии 7.0.

Шаг 1

Отметьте свойства как устаревшие с помощью ObsoleteAttribute и передайте true для параметра error. 

[Obsolete("Use pattern matching instead.", true)]
public NullToolDataValue NullData => this as NullToolDataValue;

[Obsolete("Use pattern matching instead.", true)]
public DumbellDataValue DumbellData => this as DumbellDataValue;

[Obsolete("Use pattern matching instead.", true)]
public HeartRateDataValue HRData => this as HeartRateDataValue;

[Obsolete("Use pattern matching instead.", true)]
public SomeForceDataValue SomeForceData => this as SomeForceDataValue;

Это сделает ошибкой компилятора использование их в любом коде, обработанном компилятором. Если вы поразмышляете о них, вы получите вместо этого исключение времени выполнения (после выполнения шага 3), если вы также не измените этот код.

Шаг 2

Измените каждый сайт вызовов, который использует эти свойства, чтобы использовать сопоставление с шаблоном. Если все, что вы делаете, это то, что вы показали в вопросе, это должно быть так просто: 

public class DumbellExcercise
{
    public void ToolDataReceived(ToolData data)
    {
        if (data is DumbellDataValue dumbell)
            Collection.Add(dumbell.WeightValue);

        // OR

        if (!(data is DumbellDataValue dumbell))
            return;

        Collection.Add(dumbell.WeightValue);
    }
}

Второй вариант не так хорош, потому что условие должно быть заключено в скобки, прежде чем оно может быть отменено (эй, по крайней мере, у VB есть ключевое слово IsNot; go figure), но вы получаете тот же ранний возврат, что и у существующего кода .

Похоже, вы используете по крайней мере C # 6.0, потому что вы используете оператор объединения нулей (?.), но если вы не используете по крайней мере 7.0, вы можете сделать это вместо этого: 

public class DumbellExcercise
{
    public void ToolDataReceived(ToolData data)
    {
        DumbellDataValue dumbell = data as DumbellDataValue;
        if (dumbell != null)
            Collection.Add(dumbell.WeightValue);

        // OR

        DumbellDataValue dumbell = data as DumbellDataValue;
        if (dumbell == null)
            return;

        Collection.Add(dumbell.WeightValue);
    }
}

Шаг 3

Удалить свойства. Если ошибок компиляции больше нет, свойства не используются, поэтому вы можете избавиться от них.

Дополнительное примечание

Свойство IsValid имеет странную двойственность. Он может быть назначен производными классами, но он также является виртуальным, поэтому его также можно переопределить. Вы действительно должны выбрать один. Если бы это было мое решение, я бы оставил его виртуальным и сделал бы его доступным только для чтения. 

public abstract class ToolData
{
    // Continue to assume it's true...
    public virtual bool IsValid => true;
}

public class NullToolDataValue : ToolData
{
    // ...and indicate otherwise as needed.
    public override bool IsValid => false;
}
0 голосов
/ 18 апреля 2019

Хорошо, я собираюсь дать ответ на вопрос - надеюсь, это поможет.

Во-первых, ToolData не должна содержать никаких ссылок / перечислений / независимо от того, что перечисляет свои подтипы.Итак, сначала на разделочной доске: все лямбда-свойства, которые приводят объект к определенному подтипу.Я могу понять апелляцию - вы знаете экземпляр ToolType оказался FloobieTool, поэтому вы вызываете instance.FloobieTool и волшебным образом получаете приведение FloobieTool.Но ... ну, есть проблемы, которые идут с этим, не в последнюю очередь это то, что вы нарушаете открытый / закрытый принцип.Нет ничего плохого в том, чтобы заставить человека, вызывающего класс, явным образом привести его к экземпляру (FloobieTool), если они знают, что работают с FloobieTool.

Далее: ToolType.Зачем тебе это нужно?Вы можете сообщить , если ваш экземпляр ToolData является FloobieTool, просто выполнив проверку 'is' в условии IF: 

void SomeFunc(ToolData toolData)
{
    if (!(toolData is FloobieTool)) throw new Exception("Non-Floobie!");
    // more code
}

Я имею в виду, что на самом деле дает вам это перечисление?Потому что он имеет определенную стоимость: его необходимо синхронизировать со списком классов, реализующих ToolData.

Кроме того, часть в ToolDataReceived () для каждого из этих классов Exercise кажется ... странной.Я имею в виду, у вас есть упражнение, и вы переходите в ToolData.Почему вы храните сумму упражнения Dumbell?В отличие от простого хранения ToolData.Я имею в виду, что вы проходите немало испытаний / кастований и т. Д., Просто чтобы добавить гантель вес в коллекцию.По какой-то причине вы не можете просто сохранить экземпляр ToolData и вызвать его за день?Если вам действительно нужно специально хранить информацию о Dumbbell, вы можете сделать что-то вроде: 

public class DumbbellExercise
{
    List<DumbbellDataValue> dumbbellData = new List<DumbbellDataValue>();
    public void AddToolData(ToolData toolData)
    {
        if (toolData is DumbbellDataValue)
            this.dumbbellData.Add((DumbbellDataValue)toolData);
    }
}

Надеюсь, это поможет - сложно вдаваться в слишком много деталей, когда мы работаем надПример вашей актуальной проблемы: -)

...