2012-09-12 15 views
0

下面给出了简单的代码块,我想知道是否有一个更好的办法也许代码在C#更好的实现代码替代变量选择

 int lowIndex = 0; 
     int highIndex = 1; 
     if (end[0].X.ConvertToMillimetres() == end[1].X.ConvertToMillimetres()) 
     { 
      if (end[0].Y.ConvertToMillimetres() > end[1].Y.ConvertToMillimetres()) 
      { 
       lowIndex = 1; 
       highIndex = 0; 
      } 
     } 
     else 
     { 
      if (end[0].X.ConvertToMillimetres() > end[1].X.ConvertToMillimetres()) 
      { 
       lowIndex = 1; 
       highIndex = 0; 
      } 
     } 
+2

[codereview.se] – Adam

+0

为什么你需要转换为毫米来比较值?似乎多余。你自己也说过这是一个简单的代码块。因此是否需要使它更简单?可读性是国王。 –

+0

[标题不需要标签](http://meta.stackexchange。com/questions/19190/should-questions-include-tags-in-titles) – Default

回答

4

如何像

int lowIndex = 0; 
int highIndex = 1; 
if ((end[0].X.ConvertToMillimetres() == end[1].X.ConvertToMillimetres() && end[0].Y.ConvertToMillimetres() > end[1].Y.ConvertToMillimetres()) || 
    (end[0].X.ConvertToMillimetres() > end[1].X.ConvertToMillimetres())) 
{ 
    lowIndex = 1; 
    highIndex = 0; 
} 
+0

我已经写了完全相同的代码块,但你几分钟就打败了我。 –

1

这样的事情:

int X0mm = end[0].X.ConvertToMillimetres(); 
int X1mm = end[1].X.ConvertToMillimetres(); 
int Y0mm = end[0].Y.ConvertToMillimetres(); 
int Y1mm = end[1].Y.ConvertToMillimetres(); 

int lowIndex = (X0mm == X1mm && Y0mm > Y1mm) || (X0mm > X1mm) ? 1 : 0; 
int highIndex = lowIndex == 1 ? 0 :1; 
+0

正如写入'lowIndex'和'highIndex'的默认值从不使用。您可以在底部定义这些整数,而不是使代码更加紧凑。 –

+0

@ScottLemmon是的,你是对的。 – Reniuz

+0

@ScottLemmon - 再看一眼... – sweetfa

1

我认为你想要做的是消除有两条线设置lowIndexhighIndex两次。你可以结合这样的IF陈述。

int lowIndex = 0; 
int highIndex = 1; 
if ((end[0].X.ConvertToMillimetres() == end[1].X.ConvertToMillimetres() && 
     end[0].Y.ConvertToMillimetres() > end[1].Y.ConvertToMillimetres()) || 
     end[0].X.ConvertToMillimetres() > end[1].X.ConvertToMillimetres()) 
{ 
    lowIndex = 1; 
    highIndex = 0; 
} 
0

不要以为这是C#4.0的,但你可以使它更具可读性:

var endOne = end[0]; 
var endTwo = end[1]; 

//now, if you would override the == operator for the type of X and Y to compare using ConvertToMillimetres(), you can have something like: 

int lowIndex = (endOne.X == endTwo.X && endOne.Y > endTwo.Y) || (endOne.X > endTwo.X) ? 1 : 0; 
int highIndex = lowIndex == 1 ? 0 : 1; 
1

肯定:

int lowIndex = 0; 
int highIndex = 1; 
if ( end[0].X.ConvertToMillimetres() == end[1].X.ConvertToMillimetres() 
    && end[0].Y.ConvertToMillimetres() > end[1].Y.ConvertToMillimetres() 
    || end[0].X.ConvertToMillimetres() != end[1].X.ConvertToMillimetres() 
    && end[0].X.ConvertToMillimetres() > end[1].X.ConvertToMillimetres()) 
{ 
    lowIndex = 1; 
    highIndex = 0; 
} 

end[0].X.ConvertToMillimetres() != end[1].X.ConvertToMillimetres() && end[0].X.ConvertToMillimetres() > end[1].X.ConvertToMillimetres()总是会等同于end[0].X.ConvertToMillimetres() > end[1].X.ConvertToMillimetres(),所以因此:

int lowIndex = 0; 
int highIndex = 1; 
if ( end[0].X.ConvertToMillimetres() == end[1].X.ConvertToMillimetres() 
    && end[0].Y.ConvertToMillimetres() > end[1].Y.ConvertToMillimetres() 
    || end[0].X.ConvertToMillimetres() > end[1].X.ConvertToMillimetres()) 
{ 
    lowIndex = 1; 
    highIndex = 0; 
} 

最后,我不确定ConvertToMillimetres的结果是什么或它是多么复杂/如果ConvertToMillimetres是时间密集型的以使用一些局部变量来捕获这些方法的值以减少计算,那么可能是有意义的......然后再次如果不是这样,则可能不值得污染你的本地范围,节省一点时间。可能,这是一个相当微不足道的功能,所以它不会很有优势。 (结尾[0]和结尾1可能会更好地作为局部变量,因为克里希纳说,甚至结束1 .X和结束1.Y等,但如果你这样做,不妨保存结果。)

//capture values 

var end0Xm = end[0].X.ConvertToMillimetres(); 
var end1Xm = end[1].X.ConvertToMillimetres(); 
var end0Ym = end[0].Y.ConvertToMillimetres(); 
var end1Ym = end[1].Y.ConvertToMillimetres(); 

//define proper lowIndex, highIndex 
int lowIndex = 0; 
int highIndex = 1; 
if ( end0Xm == end1Xm 
    && end0Ym > end1Ym 
    || end0Xm > end1Xm) 
{ 
    lowIndex = 1; 
    highIndex = 0; 
} 

这可能是保存测试以备将来使用的结果是有用的,也即消除了,如果块,这给少了机会为别人弄乱的未来。但是,你仍然必须有条件地做一些事情。下一个代码块假定您知道并了解C#'s ternary operator

var end0Xm = end[0].X.ConvertToMillimetres(); 
var end1Xm = end[1].X.ConvertToMillimetres(); 
var end0Ym = end[0].Y.ConvertToMillimetres(); 
var end1Ym = end[1].Y.ConvertToMillimetres(); 

//define proper lowIndex, highIndex 
bool testCase = (end0Xm == end1Xm 
    && end0Ym > end1Ym 
    || end0Xm > end1Xm); 

int lowIndex = testCase? 1 : 0; 
int highIndex = testCase? 0 : 1; 

或者,也许你喜欢highIndex = !testcase? 1: 0,甚至highIndex = 1 - lowIndex

等等等等。

0

我会做的方法:(代码可维护性)

private void GetValueM(List<EndType> end,out int lowIndex,out int highIndex) 
    { 
     lowIndex = 0; 
     highIndex = 1; 

     if ((end != null) && (end.Count > 2)) 
     { 
      var x0 = end[0].X; 
      var x1 = end[1].X; 
      var y0 = end[0].Y; 
      var y1 = end[1].Y; 

      if (x0 != null && x1 != null && y0 != null && y1 != null) 
      { 
       if ((x0.ConvertToMillimetres() == x1.ConvertToMillimetres() && y0.ConvertToMillimetres() > y1.ConvertToMillimetres()) || 
        (x0.ConvertToMillimetres() > x1.ConvertToMillimetres())) 
       { 
        lowIndex = 1; 
        highIndex = 0; 
       } 

      } 
      else 
      { 
       //Any is null set your value or throw exception 
      } 
     } 


    } 
1

我喜欢的可读性过紧凑的代码! :) 重命名变量,因为它最适合你的代码...

int xComparison = end[0].X.ConvertToMillimetres().CompareTo(end[1].X.ConvertToMillimetres()); 
int yComparison = end[0].Y.ConvertToMillimetres().CompareTo(end[1].Y.ConvertToMillimetres()); 

bool isMatch = ((xComparison == 0 && yComparison > 0) || xComparison > 0); 

int lowIndex = (isMatch ? 1 : 0); 
int highIndex = (isMatch ? 0 : 1);