Как вы пишете код, чья логика защищена от будущих дополнительных перечислений? - PullRequest
19 голосов
/ 30 сентября 2010

Мне трудно описать эту проблему. Может быть, поэтому мне трудно найти хорошее решение (слова просто не сотрудничают). Позвольте мне объяснить через код:

// original code
enum Fruit
{ 
    Apple,
    Orange,
    Banana,
}

...

Fruit fruit = acquireFruit();
if (fruit != Fruit.Orange && fruit != Fruit.Banana)
    coreFruit();
else
    pealFruit();
eatFruit();

Теперь притворимся, что годы развития идут с этими тремя типами. Различные разновидности вышеуказанной логики распространяются в хранимых процедурах, пакетах служб SSIS, приложениях Windows, веб-приложениях, приложениях Java, сценариях Perl и т. Д. ...

Наконец:

// new code
enum Fruit
{ 
    Apple,
    Orange,
    Banana,
    Grape,
}

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

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

Я сделал снимок в темноте:

# 1 Избегайте «Не в логике», как это

// select fruit that needs to be cored
select Fruit from FruitBasket where FruitType not in(Orange, Banana)

# 2 При необходимости используйте тщательно сконструированные методы NotIn ()

internal static class EnumSafetyExtensions
{
    /* By adding enums to these methods, you certify that 1.) ALL the logic inside this assembly is aware of the
     * new enum value and 2.) ALL the new scenarios introduced with this new enum have been accounted for.
     * Adding new enums to an IsNot() method without without carefully examining every reference will result in failure. */

    public static bool IsNot(this SalesOrderType target, params SalesOrderType[] setb)
    {
        // SetA = known values - SetB

        List<SalesOrderType> seta = new List<SalesOrderType>
        {
            SalesOrderType.Allowance,
            SalesOrderType.NonAllowance,
            SalesOrderType.CompanyOrder,
            SalesOrderType.PersonalPurchase,
            SalesOrderType.Allotment,
        };
        setb.ForEach(o => seta.Remove(o));

        // if target is in SetA, target is not in SetB
        if (seta.Contains(target))
            return true;

        // if target is in SetB, target is not not in SetB
        if (setb.Contains(target))
            return false;
        // if the target is not in seta (the considered values minus the query values) and the target isn't in setb
        // (the query values), then we've got a problem.  We've encountered a value that this assembly does not support.

        throw new InvalidOperationException("Unconsidered Value detected: SalesOrderType." + target.ToString());
    }
}

Теперь я могу безопасно использовать код, подобный этому:

bool needsCoring = fruit.IsNot(Fruit.Orange, Fruit.Banana);

Если этот код будет распространен по всей системе, исключения будут выброшены, когда Виноградная Лета прибудет в город (qa поймает их всех).

Так или иначе, это план. Кажется, что проблема должна быть очень распространенной, но я не могу найти что-либо в Google (возможно, по моей вине).

Как вы все справляетесь с этим?

UPDATE:

Я чувствую, что ответом на эту проблему является создание механизма «лови все остальное», который останавливает обработку и предупреждает тестировщиков и разработчиков о том факте, что новое перечисление требует рассмотрения. "switch ... default" отлично подходит, если он у вас есть.

Если в C # нет переключателя ... по умолчанию, мы можем исправить приведенный выше код следующим образом:

Fruit fruit = acquireFruit();
if (fruit != Fruit.Orange && fruit != Fruit.Banana)
    coreFruit();
else if(fruit == Fruit.Apple)
    pealFruit();
else
    throw new NotSupportedException("Unknown Fruit:" + fruit)
eatFruit();

ПРЕДУПРЕЖДЕНИЕ:

Вы действительно не должны использовать ни один из указанных выше псевдокодов. Он может (?) Компилироваться или даже работать, но на самом деле это ужасный код. Я видел много хороших решений в этой теме, если вы ищете подход на основе ООП. Хорошее решение, конечно, помещает все переключения и проверки в централизованный метод (заводской метод - это то, что меня поражает). Кроме того, потребуется также сверка кода.

Ответы [ 10 ]

10 голосов
/ 30 сентября 2010

Если я правильно понял ваш вопрос, наиболее распространенная практика - бросить NotSupportedException или NotImplementedException.

switch (fruit.Kind) {
case Fruit.Apple:
    Bite(fruit);
    break;
case Fruit.Banana:
    FeedToMonkey(fruit);
    break;
default: throw new NotSupportedException("Unknown fruit.");
}

Что касается добавления новых значений перечисления, которые сломали бы существующие , если -логика not-is , я считаю, что использование enum - плохой выбор в этом случае.Ваши вещи явно отличаются по поведению, они не похожи, например, на цвета.Возможно, лучше всего сделать опции ответственными за решение, как их следует обрабатывать .Тогда вы должны заменить перечисления полиморфизмом.

9 голосов
/ 30 сентября 2010

Я бы использовал типы, а не перечисления, для структур данных ... Е.Г. Создайте интерфейс IFruit, который имеет следующее:

interface IFruit
{
     bool NeedsCoring();
     void GetEaten(Person by);
     // etc.
}

И тогда я бы просто вызвал уже существующие методы для определения того, нужно ли это ядро ​​или еще что-то.

5 голосов
/ 30 сентября 2010

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

Мне кажется, что проблема заключается во введении нового типа данных.Возможно, вы захотите подумать о моделировании ваших классов, используя своего рода шаблон посетителя, особенно потому, что этот шаблон предназначен для связанных объектов с фиксированным числом четко определенных типов данных:

public abstract class Fruit {
    public abstract T Match(Func<Apple, T> f, Func<Banana, T> g, Func<Grape, T> h);

    public class Apple {
        // apple properties
        public override T Match(Func<Apple, T> f, Func<Banana, T> g, Func<Grape, T> h) {
            return f(this);
        }
    }
    public class Banana {
        // banana properties
        public override T Match(Func<Apple, T> f, Func<Banana, T> g, Func<Grape, T> h) {
            return g(this);
        }
    }
    public class Grape {
        // grape properties
        public override T Match(Func<Apple, T> f, Func<Banana, T> g, Func<Grape, T> h) {
            return h(this);
        }
    }
}

Использование:

public void EatFruit(Fruit fruit, Person p)
{
    // prepare fruit
    fruit.Match(
        apple => apple.Core(),
        banana => banana.Peel(),
        grape => { } // no steps required to prepare
        );

    p.Eat(fruit);
}

public FruitBasket PartitionFruits(List<Fruit> fruits)
{
    List<Apple> apples = new List<Apple>();
    List<Banana> bananas = new List<Banana>();
    List<Grape> grapes = new List<Grape>();

    foreach(Fruit fruit in fruits)
    {
        // partition by type, 100% type-safe on compile,
        // does not require a run-time type test
        fruit.Match(
            apple => apples.Add(apple),
            banana => bananas.Add(banana),
            grape => grapes.Add(grape));
    }

    return new FruitBasket(apples, bananas, grapes);
}

Этот стиль полезен по трем причинам:

  • Проверка будущего: Допустим, я добавил тип Pineapple идобавьте его в мой Match метод: Match(..., Func<Pineapple, T> k);.Теперь у меня есть куча ошибок компиляции, потому что все текущие использования Match проходят в 3 параметра, но мы ожидаем 4. Код не компилируется, пока не исправит все использования Match для обработки вашего нового типа - это делаетневозможно ввести новый тип, рискуя быть не обработанным в вашем коде.

  • Безопасность типов : оператор Match предоставляет вам доступ к определенным свойствамподтипов без проверки типа во время выполнения.

  • Refacturable : если вам не нравятся делегаты, как показано выше, или у вас есть несколько десятков типов иНе хочу обрабатывать их все, это очень легко обернуть эти делегаты классом FruitVisitor, поэтому каждый подтип передает себя соответствующему методу, как FruitVisitor.

4 голосов
/ 30 сентября 2010

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

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

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

Однако, если вы используете перечисление, чтобы решить, следует ли вам выполнять дополнительные специализированные операции поверх поведения по умолчанию, то это не обязательно такчто неожиданные перечисления - конец света.

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

2 голосов
/ 30 сентября 2010

Хотя у меня нет опыта C #, будь я на Java, вместо этого я бы создал интерфейс IPeelable и еще один ICoreable, и чтобы классы Fruit реализовали их.Затем, вместо того, чтобы избегать логики not, вы можете просто проверить, реализует ли полученный вами фрукт какой-либо из интерфейсов - таким образом, вы можете добавить будущие фрукты, которые реализуют как очищаемые, так и ядро, такие как мускусная дыня.

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

Используйте социальный фактор!

enum Fruit
{ 
    Apple,
    Orange,
    Banana,
    // add Grape here, and I'll shoot you
    // not kidding.
}

Со мной это сработает (то есть заставит меня изучить внутренний дизайн приложения достаточно глубоко, чтобы не вносить изменения, основанные только на «легких» предположениях):))

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

У многих есть хорошие предложения, но позвольте мне добавить еще один, который не требует полного изменения кода или поддержки объектов / классов в местах (например, sql), которые явно не поддерживают такие вещи. 1001 *

Вы заявили:

Fruit fruit = acquireFruit();
if (fruit != Fruit.Orange && fruit != Fruit.Banana)
    coreFruit();
else
    pealFruit();
eatFruit();

Что совершенно неожиданно сломается, если ввести виноград. Я думаю, что лучший подход будет:

Fruit fruit = acquireFruit();
Boolean fruitPrepared = false;

if (fruit == Fruit.Orange || fruit == Fruit.Banana) {
    pealFruit();
    fruitPrepared = true;
}

if (fruit == Fruit.Apple) {
    coreFruit();
    fruitPrepared = true;
}

if (!fruitPrepared) 
  throw new exception();

eatFruit();

Третий подход очень похож:

Fruit fruit = acquireFruit();

switch(fruit) {
  case Fruit.Orange:
  case Fruit.Banana:
    pealFruit();
    break;    
  case Fruit.Apple:
    coreFruit();
    break;
  default:
    throw new exception('unknown fruit detected');
    break;
}

Каждое из вышеперечисленных нарушается четко определенными способами, когда вы вышли за пределы того, для чего вы явно закодированы. Главное, что нужно убрать, это то, что вы явно делаете что-то для известного условия, а не для того, чтобы по умолчанию выполнить что-то в неизвестном состоянии. Вероятно, есть лучший способ выразить это.

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

Вы задаете неправильные вопросы. Вы ищите обходные пути кодирования для неадекватного дизайна. Вы хотите добавить в свой список с минимальными хлопотами.

Мне нравится ваша идея использовать enum для типов фруктов, но я бы использовал это как поле в классе Fruit.

Я бы использовал класс, а не интерфейс. Вы хотите захватить понятие о фрукте. Это конкретная вещь. OTOH интерфейс был бы хорош, если вы хотите добавить поведение или «качество», чтобы быть «плодотворным». Вы хотите иметь много разных типов «Фруктов» (класс), но вы не добавляете «фруктовые способности» (интерфейс) к другим вещам, не являющимся фруктами.

Вместо того, чтобы иметь базовый класс "Fruit" и подклассы для каждого вида фруктов, просто создайте переменную экземпляра для типа - и сделайте его перечислением. В противном случае количество подклассов может выйти из-под контроля. Теперь каждый вид - это «Фрукт». Поле «type» сообщает нам, какого рода.

Теперь, когда у нас есть фундаментальная идея класса Fruit, добавьте идею очистки / ядра в качестве другого поля. Если есть только эти два варианта, возможно, поле может быть логическим, «isPeelable». Если там, где или может быть в будущем, другие варианты, такие как «smash» или «pluck», теперь перечисление может быть хорошей идеей, так же как и для поля типа фрукт. Я полагаю, поле экземпляра класса может называться "prepToEat"?

Отсюда становится интересно. Ваш дизайн должен быть гибким для размещения новых видов фруктов. Кроме того, похоже, у вас есть метод для каждого значения "prepToEat". И у нас не будет ничего из того дерьма «кодирования исключений», которое вы описали в # 1, # 2 выше.

Поскольку у каждого фрукта есть несколько динамических частей, мы создадим фабричный класс. Это объединяет все детали для создания всех различных типов - и что более важно, будущих изменений кода - в один класс.

КЛЮЧЕВОЙ ЭЛЕМЕНТ ДИЗАЙНА использует делегат для метода "prepToEat". Это опять-таки лишает нас возможности изменять класс Fruit напрямую, когда мы добавляем фрукты в наш репитор.

  public class FruitEater
  {
     ArrayList<Fruit> myFruit;
     FruitFactory myFruitMaker;

     public FruitEater()
     {
        this.myFruit = new Arraylist();
        this.myFruitMaker = new FruitFactory();
     }

     public static void Main( args[] stuff )
     {
        myFruit.Add( myFruitMaker.Create( FruitType.Orange ));
        myFruit.Add( myFruitMaker.Create( FruitType.Apple ));

        foreach ( Fruit a in myFruit )
        {
           a.eat(); //FINALLY!!!!
        }
     }

  } //FruitEater class



  public class Fruit
  {
     public delegate void PrepareToEatDelegate();

     protected FruitType type;
     protected PrepType prepType;
     // pretend we have public properties to get each of these attributes


     // a field to hold what our delegate creates.
     private PrepareToEatDelegate prepMethod;

     // a method to set our delegate-created field
     public void PrepareToEatMethod( PrepareToEatDelegate clientMethod )
     {
        prepMethod = clientMethod;
     }

     public void Eat()
     {
        this.prepMethod();
        // do other fruit eating stuff
     }

      public Fruit(FruitType myType )
      {
        this.type = myType;
      }
  }

  public class FruitFactory
  {
     public FruitFactory() { }

     public Fruit Create( FruitType myType )
     {
        Fruit newFruit = new Fruit (myType);

        switch ( myType )
        {
           case FruitType.Orange :
              newFruit.prepType = PrepType.peel;
              newFruit.PrepareToEatMethod(new Fruit.PrepareToEatDelegate(FruitFactory.PrepareOrange));
              break;

           case FruitType.Apple :
              newFruit.prepType = PrepType.core;
              newFruit.PrepareToEatMethod( new Fruit.PrepareToEatDelegate( FruitFactory.PrepareApple ) );
              break;

           default :
              // throw an exception - we don't have that kind defined.
        }
        return newFruit;
     }// Create()

     // we need a prep method for each type of fruit this factory makes
     public static void PrepareOrange()
     {
        // whatever you do
     }

     public static void PrepareApple()
     {
        // apple stuff 
     }
  }// FruitFactory

  public enum FruitType
  {
     Orange
     ,Apple
     ,Grape
  }


  public enum PrepType
  {
     peel
     ,core
     ,pluck
     ,smash
  }
1 голос
/ 30 сентября 2010

Вообще говоря, условная логика, основанная на типах (перечислимых или регулярных), вот-вот сломается. При добавлении нового типа компилятор не заставляет вас обновлять переключатель.

В этом примере вместо размещения большей части моей логики в переключателе я бы использовал полиморфизм. Я передавал бы перечисление фабричному методу / классу, возвращал интерфейс фруктов (или базовый тип фруктов) и выполнял виртуальные методы на интерфейсе (/ базовый тип). Я должен использовать переключатель на фабрике, потому что нет другого способа реализовать это, который не является логически идентичным. Но поскольку я возвращаю абстрактный базовый класс, мне нужно сделать это только в одном месте. Я обязательно вымыть руки потом:)

using System;

enum FruitType
{
    Apple,
    Banana,
    Pineapple,
}

interface IFruit
{
    void Prepare();
    void Eat();
}

class Apple : IFruit
{
    public void Prepare()
    {
        // Wash
    }

    public void Eat()
    {
        // Chew and swallow
    }
}

class Banana : IFruit
{
    public void Prepare()
    {
        // Peel
    }

    public void Eat()
    {
        // Feed to your dog?
    }
}

class Pineapple : IFruit
{
    public void Prepare()
    {
        // Core
        // Peel
    }

    public void Eat()
    {
        // Cut up first
        // Then, apply directly to the forehead!
    }
}

class FruitFactory
{
    public IFruit GetInstance(FruitType fruitType)
    {
        switch (fruitType)
        {
            case FruitType.Apple:
                return new Apple();
            case FruitType.Banana:
                return new Banana();
            case FruitType.Pineapple:
                return new Pineapple();
            default:
                throw new NotImplementedException(
                    string.Format("Fruit type not yet supported: {0}"
                        , fruitType
                    ));
        }
    }
}

class Program
{
    static FruitType AcquireFruit()
    {
        // Todo: Read this from somewhere.  A database or config file?
        return FruitType.Pineapple;
    }

    static void Main(string[] args)
    {
        var fruitFactory = new FruitFactory();
        FruitType fruitType = AcquireFruit();
        IFruit fruit = fruitFactory.GetInstance(fruitType);
        fruit.Prepare();
        fruit.Eat();
    }
}

Причина, по которой я выбрал дизайн Prepare, а не дизайн Core / Peel / Deseed / Dehusk / Chill / Cut, заключается в том, что для каждого фрукта требуется разная подготовка. С дизайном, который разделяет методы подготовки, вам придется поддерживать весь вызывающий код (и, возможно, каждую реализацию) каждый раз, когда вы добавляете новый класс с различными требованиями. С дизайном, который скрывает конкретные детали приготовления, вы можете поддерживать каждый класс отдельно, и добавление новых фруктов не нарушает существующие.

См. Эту статью, почему мой дизайн предпочтительнее:

C ++ FAQ Lite - Виртуальные функции

1 голос
/ 30 сентября 2010

Вы не можете хранить две части данных в одном хранилище данных.Вам нужно хранить две части данных, и, следовательно, перечисление является неправильным типом данных для этого.Это должны быть экземпляры класса Fruit.

...