Skip to main content
fixed minor typos
Source Link
ChrisWue
  • 20.4k
  • 4
  • 42
  • 107

If all the Test methods look like in youthe provided sample implementation thethen I see several approaches to make your factory method cleaner:

  1. Use a switch:

     public static class Instructions
     {
         public static const string DeleteIf = "DeleteIf";
         public static const string Optional = "Optional";
         ...
     }
    
     public static PrecedentControl AsPrecedentControl(this Wd.ContentControl control)
     {
         switch (instruction.Command)
         {
             case Instructions.DeleteIf: return new DeleteIf(control);
             case Instructions.Optional: return new Optional(control);
             ...
             default:
                 throw new InvalidInstructionException(instruction);
         }
     }
    
  2. Use a data driven approach with a Dictionary:

     Dictionary<string, Type> InstructionMap = new Dictionary<string, Type> 
     {
         { Instructions.DeleteIf, typeof(DeleteIf) },
         { Instructions.Optional, typeof(Optional) },
         ...
     }
    

And your factory method being:

    public static PrecedentControl AsPrecedentControl(this Wd.ContentControl control)
    {
        Type instructionType = null;
        if (InstructionMap.TryGet(instruction.Command, out instructionType))
        {
            return Activator.CreateInstanceOf(instructionType, control);
        }
        throw new InvalidInstructionException(instruction);
    }

If by convention the instruction commands are all named the exact same as the corresponding classes dealing with them you can build the dictionary dynamically once via reflection by finding all the types deriving from PrecedentControl. Something along these lines might work:

    typeof(PrecedentControl).Assembly
                            .GetTypes()
                            .Where(t => !t.IsAbstract && typeof(PrecedentControl).IsAssignableFrom(t))
                            .ToDictionary(t => t.Name, t => t);

If all the Test methods look like in you sample implementation the I see several approaches to make your factory method cleaner:

  1. Use a switch:

     public static class Instructions
     {
         public static const string DeleteIf = "DeleteIf";
         public static const string Optional = "Optional";
         ...
     }
    
     public static PrecedentControl AsPrecedentControl(this Wd.ContentControl control)
     {
         switch (instruction.Command)
         {
             case Instructions.DeleteIf: return new DeleteIf(control);
             case Instructions.Optional: return new Optional(control);
             ...
             default:
                 throw new InvalidInstructionException(instruction);
         }
     }
    
  2. Use a data driven approach with a Dictionary:

     Dictionary<string, Type> InstructionMap = new Dictionary<string, Type> 
     {
         { Instructions.DeleteIf, typeof(DeleteIf) },
         { Instructions.Optional, typeof(Optional) },
         ...
     }
    

And your factory method being:

    public static PrecedentControl AsPrecedentControl(this Wd.ContentControl control)
    {
        Type instructionType = null;
        if (InstructionMap.TryGet(instruction.Command, out instructionType))
        {
            return Activator.CreateInstanceOf(instructionType, control);
        }
        throw new InvalidInstructionException(instruction);
    }

If by convention the instruction commands are all named the exact same as the corresponding classes dealing with them you can build the dictionary dynamically once via reflection by finding all the types deriving from PrecedentControl. Something along these lines might work:

    typeof(PrecedentControl).Assembly
                            .GetTypes()
                            .Where(t => !t.IsAbstract && typeof(PrecedentControl).IsAssignableFrom(t))
                            .ToDictionary(t => t.Name, t => t);

If all the Test methods look like in the provided sample implementation then I see several approaches to make your factory method cleaner:

  1. Use a switch:

     public static class Instructions
     {
         public static const string DeleteIf = "DeleteIf";
         public static const string Optional = "Optional";
         ...
     }
    
     public static PrecedentControl AsPrecedentControl(this Wd.ContentControl control)
     {
         switch (instruction.Command)
         {
             case Instructions.DeleteIf: return new DeleteIf(control);
             case Instructions.Optional: return new Optional(control);
             ...
             default:
                 throw new InvalidInstructionException(instruction);
         }
     }
    
  2. Use a data driven approach with a Dictionary:

     Dictionary<string, Type> InstructionMap = new Dictionary<string, Type> 
     {
         { Instructions.DeleteIf, typeof(DeleteIf) },
         { Instructions.Optional, typeof(Optional) },
         ...
     }
    

And your factory method being:

    public static PrecedentControl AsPrecedentControl(this Wd.ContentControl control)
    {
        Type instructionType = null;
        if (InstructionMap.TryGet(instruction.Command, out instructionType))
        {
            return Activator.CreateInstanceOf(instructionType, control);
        }
        throw new InvalidInstructionException(instruction);
    }

If by convention the instruction commands are all named the exact same as the corresponding classes dealing with them you can build the dictionary dynamically once via reflection by finding all the types deriving from PrecedentControl. Something along these lines might work:

    typeof(PrecedentControl).Assembly
                            .GetTypes()
                            .Where(t => !t.IsAbstract && typeof(PrecedentControl).IsAssignableFrom(t))
                            .ToDictionary(t => t.Name, t => t);
Source Link
ChrisWue
  • 20.4k
  • 4
  • 42
  • 107

If all the Test methods look like in you sample implementation the I see several approaches to make your factory method cleaner:

  1. Use a switch:

     public static class Instructions
     {
         public static const string DeleteIf = "DeleteIf";
         public static const string Optional = "Optional";
         ...
     }
    
     public static PrecedentControl AsPrecedentControl(this Wd.ContentControl control)
     {
         switch (instruction.Command)
         {
             case Instructions.DeleteIf: return new DeleteIf(control);
             case Instructions.Optional: return new Optional(control);
             ...
             default:
                 throw new InvalidInstructionException(instruction);
         }
     }
    
  2. Use a data driven approach with a Dictionary:

     Dictionary<string, Type> InstructionMap = new Dictionary<string, Type> 
     {
         { Instructions.DeleteIf, typeof(DeleteIf) },
         { Instructions.Optional, typeof(Optional) },
         ...
     }
    

And your factory method being:

    public static PrecedentControl AsPrecedentControl(this Wd.ContentControl control)
    {
        Type instructionType = null;
        if (InstructionMap.TryGet(instruction.Command, out instructionType))
        {
            return Activator.CreateInstanceOf(instructionType, control);
        }
        throw new InvalidInstructionException(instruction);
    }

If by convention the instruction commands are all named the exact same as the corresponding classes dealing with them you can build the dictionary dynamically once via reflection by finding all the types deriving from PrecedentControl. Something along these lines might work:

    typeof(PrecedentControl).Assembly
                            .GetTypes()
                            .Where(t => !t.IsAbstract && typeof(PrecedentControl).IsAssignableFrom(t))
                            .ToDictionary(t => t.Name, t => t);