工作中发现一个代码重构的例子

上个月做了一个ListBox的内容上下移动，有段代码之前看着一直不爽，后来改了又改，现在总算看得顺眼点了，所以想拿出来跟大家探讨下。并且可以用来做笔试题。

开始的代码是这样的:

classRecodeExample
{
publicobject[]Pages
{
get;
set;
}

publicvoidMoveUpPage(intselectedIndex=1)
{
if(selectedIndex==0||selectedIndex==LocalConfiguration.Pages.Count())
{
return;
}

object[]localItems=LocalConfiguration.Pages;
object[]groupItems=GroupConfiguration.Pages;

if(selectedIndex<localItems.Count())
{
List<object>pages=localItems.ToList();

//Updatethelistoflocalconfigitem.
objectselectedPage=pages[selectedIndex];
pages.RemoveAt(selectedIndex);
pages.Insert(selectedIndex-1,selectedPage);

localItems=pages.ToArray();
}
else
{
List<object>ticker=groupItems.ToList();

//Updatethelistofgroupconfigitem.
intposition=selectedIndex-localItems.Count();

objectselectedTicker=ticker[position];
ticker.RemoveAt(position);
ticker.Insert(position-1,selectedTicker);

groupItems=ticker.ToArray();
}

LocalConfiguration.Pages=localItems;
GroupConfiguration.Pages=groupItems;
}

publicvoidMoveDownPage(intselectedIndex=1)
{
if(selectedIndex==(LocalConfiguration.Pages.Count()-1)
||selectedIndex==(LocalConfiguration.Pages.Count()+GroupConfiguration.Pages.Count()-1))
{
return;
}

object[]localItems=LocalConfiguration.Pages;
object[]groupItems=GroupConfiguration.Pages;

if(selectedIndex<localItems.Count())
{
List<object>pages=localItems.ToList();

//Updatethelistoflocalconfigitem.
objectselectedPage=pages[selectedIndex];
pages.RemoveAt(selectedIndex);
pages.Insert(selectedIndex+1,selectedPage);

localItems=pages.ToArray();
}
else
{
List<object>ticker=groupItems.ToList();

//Updatethelistofgroupconfigitem.
intposition=selectedIndex-localItems.Count();

objectselectedTicker=ticker[position];
ticker.RemoveAt(position);
ticker.Insert(position+1,selectedTicker);

groupItems=ticker.ToArray();
}

LocalConfiguration.Pages=localItems;
GroupConfiguration.Pages=groupItems;
}
}

publicclassLocalConfiguration
{
publicstaticobject[]Pages
{
get;
set;
}
}

publicclassGroupConfiguration
{
publicstaticobject[]Pages
{
get;
set;
}
}

针对上面的代码，我一个同事做了这样的重构,如下:

classRecodeVersion2:RecodeExample
{
publicvoidMoveUpPage(intselectedIndex=1)
{
object[]localMarkee=LocalConfiguration.Pages;
object[]groupMarkee=GroupConfiguration.Pages;

this.UpdateConfigList(true,selectedIndex,reflocalMarkee,refgroupMarkee);

LocalConfiguration.Pages=localMarkee;
GroupConfiguration.Pages=groupMarkee;
}

publicvoidMoveDownPage(intselectedIndex=1)
{
object[]localPages=LocalConfiguration.Pages;
object[]groupPages=GroupConfiguration.Pages;

this.UpdateConfigList(false,selectedIndex,reflocalPages,refgroupPages);

LocalConfiguration.Pages=localPages;
GroupConfiguration.Pages=groupPages;
}

///<summary>
///Moveupormovedowntheselectedconfigurationitem.
///</summary>
///<paramname="isMoveUp">Ifmoveupitwillbetrueorfalse.</param>
///<paramname="selectedIndex">Theindexoftheselectediteminthelistbox.</param>
///<paramname="localItems">Localconfiguration.</param>
///<paramname="groupItems">Groupconfiguration.</param>
privatevoidUpdateConfigList(boolisMoveUp,intselectedIndex,refobject[]localItems,refobject[]groupItems)
{
intstep=-1;
if(!isMoveUp)
{
step=1;
}

//Theedgeitemwillnottobemovedupormovedown.
if(isMoveUp)
{
if(selectedIndex==0||selectedIndex==localItems.Count())
{
return;
}
}
else
{
if(selectedIndex==(localItems.Count()-1)||selectedIndex==(localItems.Count()+groupItems.Count()-1))
{
return;
}
}

if(selectedIndex<localItems.Count())
{
List<object>ticker=localItems.ToList();

//Updatethelistoflocalconfigitem.
objectselectedTicker=ticker[selectedIndex];
ticker.RemoveAt(selectedIndex);
ticker.Insert(selectedIndex+step,selectedTicker);

localItems=ticker.ToArray();
}
else
{
List<object>ticker=groupItems.ToList();

//Updatethelistofgroupconfigitem.
intposition=selectedIndex-localItems.Count();

objectselectedTicker=ticker[position];
ticker.RemoveAt(position);
ticker.Insert(position+step,selectedTicker);

groupItems=ticker.ToArray();
}
}
}

我开始做了这样的重构,如下:

classRecodeVersion3:RecodeExample
{
publicnewvoidMoveUpPage(intselectedIndex=1)
{
//Nonon-localitemscanbeportenintolocalitemlists.
if(selectedIndex==LocalConfiguration.Pages.Count())
{
return;
}

intindex=selectedIndex;
List<Object>pages=newList<Object>();

if((LocalConfiguration.Pages.Count()>0)
&&(index>LocalConfiguration.Pages.Count()))
{
index=selectedIndex-LocalConfiguration.Pages.Count();
}

if(selectedIndex<LocalConfiguration.Pages.Count())
{
pages=MoveUpReset(index,LocalConfiguration.Pages.ToList());
LocalConfiguration.Pages=pages.ToArray();
}
else
{
pages=MoveUpReset(index,GroupConfiguration.Pages.ToList());
GroupConfiguration.Pages=pages.ToArray();
}
}

publicnewvoidMoveDownPage(intselectedIndex=1)
{
//Nolocalitemscanbeportenintonon-localitems.
if((selectedIndex+1==LocalConfiguration.Pages.Count())
&&(LocalConfiguration.Pages.Count()>0))
{
return;
}

intindex=selectedIndex;
List<Object>pages=newList<Object>();

if((LocalConfiguration.Pages.Count()>0)
&&(index>LocalConfiguration.Pages.Count()))
{
index=selectedIndex-LocalConfiguration.Pages.Count();
}

if(selectedIndex<LocalConfiguration.Pages.Count())
{
pages=MoveDownReset(index,LocalConfiguration.Pages.ToList());
LocalConfiguration.Pages=pages.ToArray();
}
else
{
pages=MoveDownReset(index,GroupConfiguration.Pages.ToList());
GroupConfiguration.Pages=pages.ToArray();
}
}

privatestaticList<Object>MoveUpReset(intindex,List<Object>pages)
{
Objectitem=pages.ElementAt(index);
pages.RemoveAt(index);
pages.Insert(index-1,item);
returnpages;
}

privatestaticList<Object>MoveDownReset(intindex,List<Object>pages)
{
Objectitem=pages.ElementAt(index);
pages.RemoveAt(index);
pages.Insert(index+1,item);
returnpages;
}
}

发现虽然把业务逻辑封装了，但是代码有一个硬伤，就是数据类型的问题，LocalConfiguration.Pages的类型是数组，使用起来不方便，改成泛型，代码就好多了。

classRecodeVersion4:RecodeExample
{
publicList<object>Pages
{
get;
set;
}

publicnewvoidMoveUpPage(intselectedIndex=1)
{
Up(selectedIndex,LocalConfiguration.OverridePages,GroupConfiguration.OverridePages);
}

publicnewvoidMoveDownPage(intselectedIndex=1)
{
Down(selectedIndex,LocalConfiguration.OverridePages,GroupConfiguration.OverridePages);
}

privatevoidDown(intselectedIndex,List<Object>local,List<Object>group)
{
//Nolocalitemscanbeportenintonon-localitems.
if((selectedIndex+1==local.Count())
&&(0<local.Count()))
{
return;
}

InnerReset(selectedIndex,local,group,selectedIndex,newAction(MoveDownReset));
}

privatevoidUp(intselectedIndex,List<Object>local,List<Object>group)
{
//Nonon-localitemscanbeportenintolocalitemlists.
if(selectedIndex==local.Count())
{
return;
}

InnerReset(selectedIndex,local,group,selectedIndex,newAction(MoveUpReset));
}

privatevoidInnerReset(intselectedIndex,List<Object>local,List<Object>group,intindex,Actionaction)
{
if((0<local.Count())
&&(index>local.Count()))
{
index=index-local.Count();
}

if(selectedIndex<local.Count())
{
action(index,local);
}
else
{
action(index,group);
}
}

privatedelegatevoidAction(intindex,List<Object>items);

privatestaticvoidMoveUpReset(intindex,List<Object>pages)
{
Objectitem=pages.ElementAt(index);
pages.RemoveAt(index);
pages.Insert(index-1,item);
}

privatestaticvoidMoveDownReset(intindex,List<Object>pages)
{
Objectitem=pages.ElementAt(index);
pages.RemoveAt(index);
pages.Insert(index+1,item);
}
}

注意这里为了说明性使用，所以跟源码不完全一样。

publicclassLocalConfiguration
{
publicstaticobject[]Pages
{
get;
set;
}

publicstaticList<object>OverridePages
{
get;
set;
}
}

publicclassGroupConfiguration
{
publicstaticobject[]Pages
{
get;
set;
}

publicstaticList<object>OverridePages
{
get;
set;
}
}