Код Smell в Dynamic Expression

Question

Этот код решает интересную загадку, описанную в http://www.programgood.net/2011/01/13/DynamicOperatorsGuernseyChallenge.aspx

Проблема: Там, кажется, много повторять здесь .. СУХОЙ (Не повторяйте Yourself) принцип пружины на ум здесь. Кто-нибудь видит рефактор?

 string opZ = ""; 
     string opA = ""; 
     string opB = ""; 
     string opC = ""; 
     string opD = ""; 
     for (int h = 1; h <= 2; h++) // making the first number positive or negative 
     { 
      if (h == 1) opZ = ""; 
      if (h == 2) opZ = "-"; 

      for (int i = 1; i <= 4; i++) 
      { 
       if (i == 1) opA = "*"; 
       if (i == 2) opA = "/"; 
       if (i == 3) opA = "+"; 
       if (i == 4) opA = "-"; 
       for (int j = 1; j <= 4; j++) 
       { 
        if (j == 1) opB = "*"; 
        if (j == 2) opB = "/"; 
        if (j == 3) opB = "+"; 
        if (j == 4) opB = "-"; 
        for (int k = 1; k <= 4; k++) 
        { 
         if (k == 1) opC = "*"; 
         if (k == 2) opC = "/"; 
         if (k == 3) opC = "+"; 
         if (k == 4) opC = "-"; 
         for (int l = 1; l <= 4; l++) 
         { 
          if (l == 1) opD = "*"; 
          if (l == 2) opD = "/"; 
          if (l == 3) opD = "+"; 
          if (l == 4) opD = "-"; 
          string expression = opZ + 1 + opA + 3 + opB + 5 + opC + 7 + opD + 9; 
          DataTable dummy = new DataTable(); 
          double result = Convert.ToDouble(dummy.Compute(expression, string.Empty)); 
          if (result == 3) 
           Debug.WriteLine(expression + " = 3"); 

          if (result == 47) 
           Debug.WriteLine(expression + " = 47"); 

          if (result == 18) 
           Debug.WriteLine(expression + " = 18"); 

         } 
        } 
       } 
      } 
     } 

Answer 1

Ну, первый очевидный рефакторинг будет иметь множество операторов:

String[] operators = { null, "*", "/", "+", "-" }; 

Затем использовать:

opC = operators[j]; // etc 

(Лично я бы использовать петлю происходит от 0 до 3 вместо 1 - 4 - это более идиоматическая ИМО, но это другое дело.)

Тогда есть способ bu перестраивание перестановок. Я бы на самом деле использовать LINQ для этого:

string[] prefixes = { "", "-" }; 
string[] operators = { "*", "/", "+", "-" }; 
var expressions = from prefix in prefixes 
        from opA in operators 
        from opB in operators 
        from opC in operators 
        from opD in operators 
        select prefix + 1 + opA + 3 + opB + 5 + opC + 7 + opD + 9; 

foreach (string expression in expressions) 
{ 
    ... 
} 

Answer 2

char[] ops = new [] {'*','/','+','-'}; 
foreach(string opA in ops) 
foreach(string opB in ops) 
    foreach(string opC in ops) 
    foreach(string opD in ops) 
    foreach(string opZ in new []{'-',' '}) { 
    string expression = opZ + 1 + opA + 3 + opB + 5 + opC + 7 + opD + 9; 
    DataTable dummy = new DataTable(); 
    double result = Convert.ToDouble(dummy.Compute(expression, string.Empty)); 
    if (result == 3) 
     Debug.WriteLine(expression + " = 3"); 
    if (result == 47) 
     Debug.WriteLine(expression + " = 47"); 
    if (result == 18) 
     Debug.WriteLine(expression + " = 18"); 
    } 

Answer 3

Я полагаю, что нет никакого реального смысла делать это при использовании DataTable.Compute, но

var calculator = new DataTable() ; 
var operators = "*/+-" ; 
for (int i = 0 ; i < 0x200 ; ++i) 
{ 
    var value = calculator.Compute (String.Format ("{0}1{1}3{2}5{3}7{4}9", 
    (i & 0x100) != 0 ? "-" : "", 
    operators[(i >> 0) & 3], 
    operators[(i >> 2) & 3], 
    operators[(i >> 4) & 3], 
    operators[(i >> 6) & 3]), String.Empty) ; 
    ... 
} 

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

var opstrings = "+-*/" ; 
var operators = new Func<int, int, int>[] { 
    (a, b) => a + b, 
    (a, b) => a - b, 
    (a, b) => a * b, 
    (a, b) => a/b, } ; 
for (int i = 0 ; i < 0x200 ; ++i) 
{ 
    var stack = 0 ; // seed value 
    var last = 0 ; // imitate + for lowest precedence 
    var value =(i & 0x100) != 0 ? -1 : 1 ; 

    for (int j = 0 ; j < 5 ; ++j) // extra item to force last reduction 
    { 
    var oper = (i >> j * 2) & 3 ; // "input" operator 
    var input = 3 + j * 2 ;  // "input" number 
    if (oper/2 <= last/2)  // reduce below top? 
    { 
     stack = operators[last] (stack, value) ; 
     last = oper ;    // shift operator 
     value = input ;    // shift number 
    } 
    else       // reduce top 
     value = operators[oper] (value, input) ; 
    } 

    var result = stack ; 
    if (result == 3 || result == 47 || result == 18) 
    Debug.WriteLine ("{0}1{1}3{2}5{3}7{4}9 = {5}", 
     (i & 0x100) != 0 ? "-" : "", 
     opstrings[(i >> 0) & 3], 
     opstrings[(i >> 2) & 3], 
     opstrings[(i >> 4) & 3], 
     opstrings[(i >> 6) & 3], 
     result) ; 
}