Лучший способ рефакторинга это? - PullRequest
1 голос
/ 27 октября 2010

У меня есть следующие классы:

public abstract class BaseClass
{
    private readonly double cachedValue;

    public BaseClass()
    {
         cachedValue = ComputeValue();
    }

    protected abstract double ComputeValue()          
}


public class ClassA : BaseClass
{
    protected override double ComputeValue()  { ... }            
}

public class ClassB : BaseClass
{
    protected override double ComputeValue()  { ... }                
}

, где ComputeValue() - это трудоемкое вычисление.

Проблема в том, что в ClassA и ClassB существуют другие методы, которые требуют значения, возвращаемого из ComputeValue(). Я думал о добавлении защищенного свойства с именем CachedValue в BaseClass, но я нахожу этот подход избыточным и вводящим в заблуждение других программистов, которые могут не знать о его существовании и могут напрямую вызывать ComputeValue ().

Второй вариант - использовать обнуляемый тип на уровне производного класса, поскольку мне необязательно требовать вычисления в конструкторе в BaseClass, ленивое вычисление может быть лучшим вариантом:

protected override double ComputeValue()  
{
    if(cachedValue.HasValue)
    {
        return (double) cachedValue;
    }

    // Do computation
    cachedValue = ...

    return cachedValue;
}        

но я чувствую, что могу сделать лучше.

Что вы думаете об этом?

Спасибо.

Редактировать: Просто чтобы уточнить, я пытаюсь предотвратить повторный вызов ComputeValue(), принудительно используя 'cachedValue'.

Ответы [ 7 ]

3 голосов
/ 27 октября 2010

Почему бы не взять delagate в конструкторе, который имеет ту же цель ComputeValue, а затем выставить значение как поле protected readonly?

public abstract class BaseClass {
  protected readonly double cachedValue;

  protected BaseClass(Func<double> computeValue) {
    cachedValue = computeValue();
  }
}
2 голосов
/ 27 октября 2010

Если вы в сети 4, просто используйте Lazy<T>.

public class ClassA
{
    Lazy<double> value = new Lazy<double>(()=>something.SomethingComplicated());

    public void AnyMethod()
    {
        double d = value.Value;
        //...
    }

}

См. http://msdn.microsoft.com/en-us/library/dd642331.aspx

Обновление: так как вы работаете в .NET 3.5, вот отличная статья о реализации lazy самостоятельно (всего около 20 LOC): http://msdn.microsoft.com/en-us/vcsharp/bb870976.aspx

Как правило, хороший совет - всегда использовать композицию вместо наследования:)

2 голосов
/ 27 октября 2010

В какой-то момент вам придётся инициализировать это значение, пути к этому нет. Поэтому я думаю, что метод, в котором вы используете значение NULL, имеет смысл - я согласен, что он гораздо понятнее, чем наличие там кэшированного свойства.

Возможно, вы захотите добавить параметр в ComputeValue, который заставляет значение вычисляться снова:

protected override double ComputeValue(bool force)   
{ 
    if(!force && cachedValue.HasValue) 
    { 
        return cachedValue; 
    } 

    // Do computation 
    cachedValue = ... 

    return cachedValue; 
}         
1 голос
/ 27 октября 2010

Для хранения / кэширования вычисленного значения вы можете использовать шаблон Singleton, в основном вы объявляете статическое поле, затем проверяете нулевое значение перед попыткой вычисления. Таким образом, вычисленное значение будет вычислено только при первом вызове / добавлении метода или свойства. Если некоторый производный класс требует другого вычисляемого значения, вы переопределяете метод / свойство базового класса (виртуальный модификатор необходим для обеспечения полиморфизма). Рекомендуется использовать свойства вместо методов для элементов, представляющих тип данных для класса.

public abstract class BaseClass {
    private static double _cachedValue = null;
    public BaseClass() { }
    // base class implements a Singleton Pattern to store a computed value
    protected virtual double ComputeValue
    {
        get
        {
            if( _cachedValue == null) { _cachedValue = CalculateComputedValue; }
            return _cachedValue;
        }
    }   
    private double CalculateComputedValue() { return someComplexComputedValue; }
}

public class ClassA : BaseClass
{
    private static double _cachedValue = null;
    //this class does require calculate a specific computed value.
    protected override double ComputeValue
    { 
        get
        {
            if( _cachedValue == null) { _cachedValue = CalculateComputedValue; }
            return _cachedValue;
        }
    }
    private double CalculateComputedValue() { return someComplexComputedValue; }
}

public class ClassB : BaseClass
{
    //this class does not require to calculate compute value, inherited value is used instead.
}
1 голос
/ 27 октября 2010

Я думаю, что ComputeValue должен лениво вызываться в получателе свойств:

public abstract class BaseClass
{
    private Func<double> _computeValue;

    private double? _cachedValue;
    protected double cachedValue
    {
       get
       {
          if(_cachedValue == null)
             _cachedValue = _computeValue();
          return (double)_cachedValue;
       }
       private set
       {
          _cachedValue = value;
       }
    }

    private BaseClass(){};
    public BaseClass(Func<double> computeValue)
    {
         _computeValue = computeValue;
    }    

}
1 голос
/ 27 октября 2010

Другой подход заключается в добавлении метода открытого интерфейса GetValue в Baseclass и разрешении перезаписывать метод ComputeValue только для унаследованных классов - таким образом вы все равно можете расширить функциональность, но вы контролируете поведение контекст результата ComputeValue в вашем базовом классе, т.е. вы можете добавить памятку, как в примере, или декорировать / расширять по мере необходимости.

Это соответствует шаблону Non-Virtual interface (NVI) .

    public abstract class BaseClass
    {
        private double? cachedValue;

        public BaseClass()
        {
        }

        protected abstract double ComputeValue();

        public virtual double GetValue()
        {
            if (cachedValue.HasValue)
            {
                return (double)cachedValue;
            }
            else
            {
                cachedValue = ComputeValue();
                return (double)cachedValue;
            }

        }
    }
1 голос
/ 27 октября 2010

Существует ли соответствующая связь между логикой, которая нуждается в вычисленном значении, и логикой для вычисления значения?

Если это не так или, по крайней мере, 100% -ное совпадение, вы могли бы поставитьлогика расчета в отдельных классах: CalculatorA и CalculatorB, которые оба наследуют от интерфейса ICalculator.Базовый класс может быть единственным классом, который обращается к этому интерфейсу и кэширует результаты.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...