2010-05-21 12 views
15

Attualmente ho una dichiarazione switch che esegue circa 300 righe dispari. So che questo non è il massimo che può ottenere, ma sono sicuro che c'è un modo migliore per gestirlo.Migliore alternativa all'istruzione Case

L'istruzione switch prende uno Enum che viene utilizzato per determinare determinate proprietà che riguardano la registrazione. In questo momento il problema si pone in quanto è molto facile omettere un valore di enumerazione e che non verrà dato un valore poiché non è nell'istruzione switch.

Esiste un'opzione che è possibile utilizzare per garantire che ogni enumerazione venga utilizzata e fornita di un insieme personalizzato di valori che deve svolgere il proprio lavoro?

EDIT:


Esempio di codice come richiesto: (.. Questo è semplicistico, ma mostra esattamente quello che voglio dire anche un'enumerazione esisterebbe con i valori al di sotto)

internal void GenerateStatusLog(LogAction ActionToLog) 
{ 
    switch (ActionToLog) 
    { 
     case LogAction.None: 
      { 
       return; 
      } 
     case LogAction.LogThis: 
      { 
       ActionText = "Logging this Information"; 
       LogText = "Go for it."; 

       break; 
      } 
    } 

    // .. Do everything else 
} 
+3

Puoi fornire un esempio di codice (limitato) che mostra il tipo di lavoro svolto? –

+1

Sì, abbiamo bisogno di più codice! Inoltre, guarda questo metodo http://msdn.microsoft.com/en-us/library/system.enum.getnames.aspx –

+0

@Oskar Kjellin: +1 ma probabilmente vuoi dire Valori piuttosto che nomi? –

risposta

0

Prova a usa la riflessione.

  • Decorare le opzioni enum con gli attributi che contengono il valore associato e restituiscono questo valore.
  • Crea classe statica di costanti e l'uso di riflessione per la mappatura enum-opzione per costante per nome

hope this will help

+0

Questo potrebbe funzionare, ma sarà MOLTO lento. Usare la riflessione sull'enumerazione è un modo per non usarlo normalmente. – Venemo

+0

Non ci sono requisiti di prestazione nella domanda. Le chiamate al database di rete costano centinaia di volte di più, quindi i costi di riflessione possono essere insignificanti –

+0

Questo verrà eseguito abbastanza spesso. Non penso che questo sarà abbastanza performante. –

4

Beh, c'è gettando nel caso default ... Non c'è scrivi/compila tempo costruire altri di quello.

Tuttavia, Strategia, Visitatore e altri modelli ad essi correlati possono essere appropriati se si sceglie di farlo in fase di esecuzione.

Il codice di esempio consente di ottenere la risposta migliore.

MODIFICA: Grazie per il campione. Continuo a pensare che abbia bisogno di un po 'di rimodellamento, visto che non ci sono alcuni parametri applicabili solo ad alcuni case s ecc.

L'azione è spesso usata come alias per il pattern Command e il fatto che il tuo Enum è chiamato LogAction significa che ogni valore porta con sé un comportamento - sia quello implicito (si incolla il codice appropriato in un case) o esplicito (nella specifica classe della gerarchia di comando).

Quindi mi sembra che un utilizzo del modello Command sia appropriato (sebbene il campione non lo provi) - cioè, avere una classe (potenzialmente una gerarchia che utilizza overload di costruttori o qualsiasi altro [set di] meccanismi di fabbrica) che mantiene lo stato associato alla richiesta insieme al comportamento specifico. Quindi, invece di passare un valore Enum, creare un'istanza appropriata da LogCommand nel registratore, che la invoca semplicemente (potenzialmente passando una "presa" del Log Sink in cui il Comando può accedere). Altrimenti si stanno sottoponendo a sottoinsiemi casuali di parametri in luoghi diversi.

relativi seealso post:

+0

@downvoter spiega per favore per amorevolezza - felice di modificare ... –

0

Alcune volte la memorizzazione le opzioni in una mappa è una buona soluzione, è possibile esternare la configurazione in un file anche, non sono sicuro se si applica alla tua applicazione.

5

EDIT

Ho pensato che questo ancora una volta, si guardò intorno in questioni connesse a SO, e ho scritto un po 'di codice. Ho creato una classe denominata AdvancedSwitch<T>, che consente di aggiungere casi ed espone un metodo per valutare un valore e consente di specificare valori che deve verificare per esistenza.

Questo è ciò che mi si avvicinò con:

public class AdvancedSwitch<T> where T : struct 
{ 
    protected Dictionary<T, Action> handlers = new Dictionary<T, Action>(); 

    public void AddHandler(T caseValue, Action action) 
    { 
     handlers.Add(caseValue, action); 
    } 

    public void RemoveHandler(T caseValue) 
    { 
     handlers.Remove(caseValue); 
    } 

    public void ExecuteHandler(T actualValue) 
    { 
     ExecuteHandler(actualValue, Enumerable.Empty<T>()); 
    } 

    public void ExecuteHandler(T actualValue, IEnumerable<T> ensureExistence) 
    { 
     foreach (var val in ensureExistence) 
      if (!handlers.ContainsKey(val)) 
       throw new InvalidOperationException("The case " + val.ToString() + " is not handled."); 

     handlers[actualValue](); 
    } 
} 

si può consumare la classe in questo modo:

public enum TrafficColor { Red, Yellow, Green } 

public static void Main() 
{ 
    Console.WriteLine("Choose a traffic color: red, yellow, green?"); 
    var color = (TrafficColor)Enum.Parse(typeof(TrafficColor), Console.ReadLine()); 
    var result = string.Empty; 

    // Creating the "switch" 
    var mySwitch = new AdvancedSwitch<TrafficColor>(); 

    // Adding a single case 
    mySwitch.AddHandler(TrafficColor.Green, delegate 
    { 
     result = "You may pass."; 
    }); 

    // Adding multiple cases with the same action 
    Action redAndYellowDelegate = delegate 
    { 
     result = "You may not pass."; 
    }; 
    mySwitch.AddHandler(TrafficColor.Red, redAndYellowDelegate); 
    mySwitch.AddHandler(TrafficColor.Yellow, redAndYellowDelegate); 

    // Evaluating it 
    mySwitch.ExecuteHandler(color, (TrafficColor[])Enum.GetValues(typeof(TrafficColor))); 

    Console.WriteLine(result); 
} 

Con l'uso creativo dei delegati anonimi, è possibile aggiungere facilmente nuovi casi al vostro "interruttore". :)
Non è possibile utilizzare espressioni lambda e blocchi lambda, ad esempio () => { ... } anziché delegate { ... }.

È possibile utilizzare facilmente questa classe anziché i blocchi di commutazione lunghi.

Original post:

Se si utilizza Visual Studio, creare sempre swich bilancio con il switch frammento di codice. Digita il tasto switch due volte e genera automaticamente tutte le possibilità per te.

Quindi, aggiungere un caso default alla fine che genera un'eccezione, in questo modo quando si verifica l'app si noterà che c'è un caso non gestito, all'istante.

voglio dire qualcosa di simile:

switch (something) 
{ 
    ... 
    case YourEnum.SomeValue: 
     ... 
     break; 
    default: 
     throw new InvalidOperationException("Default case reached."); 
} 
+0

-1 Non penso che questo sia il problema. –

+1

@Oskar, perché no? Mi sembra che @Ruben descriva esattamente la stessa cosa, tuttavia ha ottenuto +2 e non -1. – Venemo

+1

@Venemo allora perché ripubblicarlo mezz'ora dopo? ;) –

2

Una possibile soluzione è quella di utilizzare un SortedDictionary:

delegate void EnumHandler (args); 
SortedDictionary <Enum, EnumHandler> handlers; 

constructor 
{ 
    handlers = new SortedDictionary <Enum, EnumHandler>(); 
    fill in handlers 
} 

void SomeFunction (Enum enum) 
{ 
    EnumHandler handler; 

    if (handlers.TryGetValue (enum, out handler)) 
    { 
    handler (args); 
    } 
    else 
    { 
    // not handled, report an error 
    } 
} 

Questo metodo non consente di sostituire i gestori in modo dinamico. Puoi anche usare un elenco come parte del valore del dizionario e avere più gestori per ogni enumerazione.

+0

alcun motivo per uno specifico _Sorted_? –

+0

@RubenBartelink: abitudine principalmente. Quando utilizzo un pattern come questo, la chiave di solito non è un'enumerazione (un tipo o una stringa per esempio). Qui, dato che la chiave è un'enumerazione, potresti usare un vettore. – Skizz

+0

ma 'SortedDictionary', se paragonato a' Dictionary' ti consente solo di enumerare in modo efficiente, non lo fa? IOW, indipendentemente da quale sia la tua chiave, entrambi si appoggiano in modo uguale a 'Equals' e' GetHashCode'. Sono d'accordo che un vettore potrebbe essere appropriato [assumendo che questa domanda abbia senso: D] –

0

lungo codice di esempio qui, e il codice generico finale è un po 'pesante (EDIT hanno aggiunto un esempio in più che elimina la necessità per le parentesi angolari a scapito di una certa flessibilità finale).

Una cosa che questa soluzione offre è una buona prestazione - non proprio buona come una semplice istruzione di commutazione, ma ogni dichiarazione di caso diventa una ricerca di dizionario e l'invocazione di metodo, quindi ancora piuttosto buona. La prima chiamata otterrà una penalizzazione delle prestazioni, tuttavia, a causa dell'uso di un generico statico che riflette sull'inizializzazione.

Creare un attributo e tipo generico come segue:

[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] 
public class DynamicSwitchAttribute : Attribute 
{ 
public DynamicSwitchAttribute(Type enumType, params object[] targets) 
{ Targets = new HashSet<object>(targets); EnumType = enumType; } 
public Type EnumType { get; private set; } 
public HashSet<object> Targets { get; private set; } 
} 

//this builds a cache of methods for a given TTarget type, with a 
//signature equal to TAction, 
//keyed by values of the type TEnum. All methods are expected to 
//be instance methods. 
//this code can easily be modified to support static methods instead. 
//what would be nice here is if we could enforce a generic constraint 
//on TAction : Delegate, but we can't. 
public static class DynamicSwitch<TTarget, TEnum, TAction> 
{ 
//our lookup of actions against enum values. 
//note: no lock is required on this as it is built when the static 
//class is initialised. 

private static Dictionary<TEnum, TAction> _actions = 
    new Dictionary<TEnum, TAction>(); 

private static MethodInfo _tActionMethod; 
private static MethodInfo TActionMethod 
{ 
    get 
    { 
    if (_tActionMethod == null) 
    { 
    //one criticism of this approach might be that validation exceptions 
    //will be thrown inside a TypeInitializationException. 
    _tActionMethod = typeof(TAction).GetMethod("Invoke", 
     BindingFlags.Instance | BindingFlags.Public); 

    if (_tActionMethod == null) 
    throw new ArgumentException(/*elided*/); 

    //verify that the first parameter type is compatible with our 
    //TTarget type. 
    var methodParams = _tActionMethod.GetParameters(); 
    if (methodParams.Length == 0) 
    throw new ArgumentException(/*elided*/); 

    //now check that the first parameter is compatible with our type TTarget 
    if (!methodParams[0].ParameterType.IsAssignableFrom(typeof(TTarget))) 
    throw new ArgumentException(/*elided*/); 
    } 
    return _tActionMethod; 
    } 
} 

static DynamicSwitch() 
{ 
    //examine the type TTarget to extract all public instance methods 
    //(you can change this to private instance if need be) which have a 
    //DynamicSwitchAttribute defined. 
    //we then project the attributes and the method into an anonymous type 
    var possibleMatchingMethods = 
    from method in typeof(TTarget). 
     GetMethods(BindingFlags.Public | BindingFlags.Instance) 
    let attributes = method.GetCustomAttributes(
     typeof(DynamicSwitchAttribute), true). 
     Cast<DynamicSwitchAttribute>().ToArray() 
    where attributes!= null && attributes.Length == 1 
     && attributes[0].EnumType.Equals(typeof(TEnum)) 
    select new { Method = method, Attribute = attributes[0] }; 

    //create linq expression parameter expressions for each of the 
    //delegate type's parameters 
    //these can be re-used for each of the dynamic methods we generate. 
    ParameterExpression[] paramExprs = TActionMethod.GetParameters(). 
    Select((pinfo, index) => 
    Expression.Parameter(
     pinfo.ParameterType, pinfo.Name ?? string.Format("arg{0}")) 
    ).ToArray(); 
    //pre-build an array of these parameter expressions that only 
    //include the actual parameters 
    //for the method, and not the 'this' parameter. 
    ParameterExpression[] realParamExprs = paramExprs.Skip(1).ToArray(); 

    //this has to be generated for each target method. 
    MethodCallExpression methodCall = null; 

    foreach (var match in possibleMatchingMethods) 
    { 
    if (!MethodMatchesAction(match.Method)) 
    continue; 

    //right, now we're going to use System.Linq.Expressions to build 
    //a dynamic expression to invoke this method given an instance of TTarget. 
    methodCall = 
    Expression.Call(
     Expression.Convert(
     paramExprs[0], typeof(TTarget) 
     ), 
     match.Method, realParamExprs); 

    TAction dynamicDelegate = Expression. 
    Lambda<TAction>(methodCall, paramExprs).Compile(); 

    //now we have our method, we simply inject it into the dictionary, using 
    //all the unique TEnum values (from the attribute) as the keys 
    foreach (var enumValue in match.Attribute.Targets.OfType<TEnum>()) 
    { 
    if (_actions.ContainsKey(enumValue)) 
    throw new InvalidOperationException(/*elided*/); 

    _actions[enumValue] = dynamicDelegate; 
    } 
    } 
} 

private static bool MethodMatchesAction(MethodInfo method) 
{ 
    //so we want to check that the target method matches our desired 
    //delegate type (TAction). 
    //The way this is done is to fetch the delegate type's Invoke 
    //method (implicitly invoked when you invoke delegate(args)), and 
    //then we check the return type and parameters types of that 
    //against the return type and args of the method we've been passed. 

    //if the target method's return type is equal to or derived from the 
    //expected delegate's return type, then all is good. 

    if (!_tActionMethod.ReturnType.IsAssignableFrom(method.ReturnType)) 
    return false; 

    //now, the parameter lists of the method will not be equal in length, 
    //as our delegate explicitly includes the 'this' parameter, whereas 
    //instance methods do not. 

    var methodParams = method.GetParameters(); 
    var delegateParams = TActionMethod.GetParameters(); 

    for (int i = 0; i < methodParams.Length; i++) 
    { 
    if (!methodParams[i].ParameterType.IsAssignableFrom(
     delegateParams[i + 1].ParameterType)) 
    return false; 
    } 
    return true; 
} 


public static TAction Resolve(TEnum value) 
{ 
    TAction result; 

    if (!_actions.TryGetValue(value, out result)) 
    throw new ArgumentException("The value is not mapped"); 

    return result; 
} 
} 

Ora fare questo in una prova di unità:

[TestMethod] 
public void TestMethod1() 
{ 
    Assert.AreEqual(1, 
    DynamicSwitch<UnitTest1, Blah, Func<UnitTest1, int>>. 
     Resolve(Blah.BlahBlah)(this)); 

    Assert.AreEqual(125, 
    DynamicSwitch<UnitTest1, Blah, Func<UnitTest1, int>>. 
     Resolve(Blah.Blip)(this)); 

Assert.AreEqual(125, 
    DynamicSwitch<UnitTest1, Blah, Func<UnitTest1, int>>. 
     Resolve(Blah.Bop)(this)); 
} 

public enum Blah 
{ 
BlahBlah, 
Bloo, 
Blip, 
Bup, 
Bop 
} 


[DynamicSwitchAttribute(typeof(Blah), Blah.BlahBlah)] 
public int Method() 
{ 
return 1; 
} 

[DynamicSwitchAttribute(typeof(Blah), Blah.Blip, Blah.Bop)] 
public int Method2() 
{ 
return 125; 
} 

Quindi, dato un valore Tenum, e il tipo preferito 'azione' (nel tuo codice sembrerebbe semplicemente non restituire nulla e modificare lo stato interno della classe), devi semplicemente consultare la classe DynamicSwitch <>, chiedergli di risolvere un metodo di destinazione e quindi invocarlo in linea (passando l'oggetto di destinazione su quale metodo verrà invocato come il primo parametro).

non mi aspettavo davvero alcun voto per questo - è un MAD soluzione ad essere onesti (che ha il vantaggio di poter essere applicato a qualsiasi tipo enum, e anche i valori discreti di tipo int/float/doppio, oltre a supportare qualsiasi tipo di delegato) - quindi forse è un po 'un martello!

EDIT

Una volta che hai un generico statico come questo, l'angolo-staffa inferno ne consegue - quindi vogliamo cercare di sbarazzarsi di loro. Molto spesso, questo viene fatto per tipo di inferenza sui parametri del metodo, ecc., Ma abbiamo un problema qui che non possiamo facilmente dedurre la firma di un delegato senza ripetere la chiamata al metodo, ad esempio (args) => return.

Tuttavia, sembra che tu abbia bisogno di un metodo che non richiede parametri e restituisce void, quindi puoi chiudere questo behemoth generico fissando il tipo di delegato su Action e lanciare una API fluidica nel mix (se questo è il tuo tipo di cosa):

public static class ActionSwitch 
{ 
    public class SwitchOn<TEnum> 
    { 
    private TEnum Value { get; set; } 

    internal SwitchOn(TEnum value) 
    { 
     Value = value; 
    } 

    public class Call<TTarget>{ 
     private TEnum Value { get; set; } 
     private TTarget Target { get; set; } 

     internal Call(TEnum value, TTarget target) 
     { 
     Value = value; 
     Target = target; 
     Invoke(); 
     } 

     internal void Invoke(){ 
      DynamicSwitch<TTarget, TEnum, Action<TTarget>>.Resolve(Value)(Target); 
     } 
    } 

    public Call<TTarget> On<TTarget>(TTarget target) 
    { 
     return new Call<TTarget>(Value, target); 
    } 
    } 

    public static SwitchOn<TEnum> Switch<TEnum>(TEnum onValue) 
    { 
    return new SwitchOn<TEnum>(onValue); 
    } 
} 

Ora aggiungere questo per il progetto di test:

[TestMethod] 
public void TestMethod2() 
{ 
    //no longer have any angle brackets 
    ActionSwitch.Switch(Blah.Bup).On(this); 

    Assert.IsTrue(_actionMethod1Called); 
} 

private bool _actionMethod1Called; 

[DynamicSwitch(typeof(Blah), Blah.Bup)] 
public void ActionMethod1() 
{ 
    _actionMethod1Called = true; 
} 

l'unico problema con questo (a parte la complessità della soluzione :)) è che ci si deve ri -build questo tipo di wrapper statico ogni volta che si desidera utilizzare un nuovo tipo di delega di destinazione te per un passaggio dinamico altrove. È possibile generare una versione generica basata su Action < ...> e Func < ...> delegati che incorporano TArg1, TArg (n) e TReturn (se Func <>) - ma si finisce per scrivere un molto più codice.

Forse trasformerò questo in un articolo sul mio blog e fare tutto questo - se avrò il tempo!

+0

+1 Interessante, se tangenziale. Dai un'occhiata ai due link che ho aggiunto alla fine del mio post. Inoltre * devi * inserire interruzioni di riga nel codice se ti aspetti che le persone lo leggano –

+1

Aggiunte alcune interruzioni di riga: hai ragione, è molto difficile leggere senza! (È difficile da leggere, suppongo) –

Problemi correlati