0

現時点では、次のコードに似た 7 つの if ステートメントがあります。

if(hit.collider.gameObject.tag == "Colour1" && start_time > look_at_time)
{
    new_colour1.ChangeObjectMaterialColour(hit.collider.gameObject.renderer.material.color);

   var colums = GameObject.FindGameObjectsWithTag("column");
   foreach( GameObject c in colums)
    c.GetComponent<MeshRenderer>().materials[1].color = new_colour1.orignalMaterial;
}


else if(hit.collider.gameObject.tag == "Colour2" && start_time > look_at_time)
{
    new_colour2.ChangeObjectMaterialColour(hit.collider.gameObject.renderer.material.color);
    var colums = GameObject.FindGameObjectsWithTag("column");
    foreach( GameObject c in colums)
       c.GetComponent<MeshRenderer>().materials[1].color = new_colour2.orignalMaterial;
}

各ステートメントは約 6 行のコードであり、多くのスペースを占有し、読みにくい場合があります。私がやりたいことは、これをリファクタリングする方法を見つけて、私のコードが少しぎこちなくなり、スペースを取りすぎないようにすることです。

if ステートメントのコレクションを switch ステートメントに変更することを考えていましたが、switch ステートメントは上記のように 2 つの引数を処理できないことがわかりました。コードをリファクタリングして同じ機能を維持できる他の方法がある場合、または if ステートメントのコレクションにこだわっていますか?

編集

私の7つのステートメントのうち2つを含むように更新されました。IFステートメントの量を減らすか、より賢い方法を見つけようとしています。これ以上余分なコード行や if ステートメントを追加したくありません。

4

2 に答える 2

2

正直なところ、あなたのケースで最も読みやすく保守しやすいのは、単純に if ブロックのロジックを関数に移動することだと思います。

  if(hit.collider.gameObject.tag == "Colour1" && start_time > look_at_time)
    {
        DoTheThing(new_colour1);
    }
if(hit.collider.gameObject.tag == "Colour2" && start_time > look_at_time)
    {
        DoTheThing(new_colour2);
    }
//and so on



 void DoTheThing(MyType newColour)
  {
 newColour.ChangeObjectMaterialColour(hit.collider.gameObject.renderer.material.color);
      var colums = GameObject.FindGameObjectsWithTag("column");
       foreach( GameObject c in colums)
        c.GetComponent<MeshRenderer>().materials[1].color = newColour.orignalMaterial;
    }

使用に関してswitchは、もちろん次のようなことができます：

switch(hit.collider.gameObject.tag)
{
  case "Colour1":
      if (start_time>look_at_time)
      {
        //do something
      }
      else if (start_time<look_at_time)
      {
        //something else
      }
      else
      {
         //they are equal, etc, etc
      }
    break;
case "Colour2":
  //and so on
}

または、すべての if ブロックが と の間start_timeで同じ比較を行う場合は、look_at_timeそれを逆にすることができます。

enum TimeDifference
{
   LessThan,Equal,GreaterThan
}
//just one way to get a constant switchable value, not necessarily the best
var diff = TimeDifference.Equal;
if (start_time>look_at_time) {diff=TimeDifference.LessThan}
else if (start_time<look_at_time) {diff=TimeDifference.GreaterThan}

switch(diff)
{
  case TimeDifference.LessThan:
     switch(hit.collider.gameObject.tag)
     {
       case "Colour1":
         //do something for Colour1
        break;   
        case "Colour2":
         //do something for Colour2
        break;
      }//end switch Colour
     break;
  case TimeDifference.GreaterThan
    switch(hit.collider.gameObject.tag)
     {
       case "Colour1":
         //do something for Colour1
        break;   
        case "Colour2":
         //do something for Colour2
        break;
      }//end switch Colour
     break;  
   default:
    //nested switch for Colour in the == case, you get the idea.
}

ただし、上記のいずれかの読みやすさと、いくつかの適切にフォーマットされた個別のifブロックの読みやすさについては、非常に議論の余地があります。

別のアプローチは、 : を使用することDictionary<string,Action<int,int>>です ( start_timeandlook_at_timeが両方ともints であり、すべてのロジックが何らかの形でそれらを含むと仮定します)

var Actions = new Dictionary<string,Action<int,int>>();
Actions.Add("Colour1",(s,l)=>{
 if (start_time>look_at_time)
          {
            //do something
          }
          else if (start_time<look_at_time)
          {
            //something else
          }
          else
          {
             //they are equal, etc, etc
          }

});

そして、あなたのメインコードは1行になります:

Actions[hit.collider.gameObject.tag](start_time,look_at_time);

そして、そのコードを再配置する方法は他にもたくさんあると確信しています。

于 2013-10-25T08:36:17.837 に答える
1

コードを更新していただきありがとうございます。これで、重複しているものを確認できます。たとえば、2 つのコード ブロックの唯一の違いは、if ステートメント内の文字列"Color1"と、および に置き換えられる変数です。"Color2"new_colour1new_colour2

ここから、次のようなものを提案します。

//This should be declared once - e.g. class level not method level.
var colorDict = new Dictionary<string, [new_color_type]> 
                    {{"Colour1", new_colour1}, {"Colour2", new_colour2}};

[new_color_type] newColor;
if(start_time > look_at_time && colorDict.TryGetValue(
        hit.collider.gameObject.tag,
        out newColor))
{
   newColor.ChangeObjectMaterialColour(
                            hit.collider.gameObject.renderer.material.color);

   var colums = GameObject.FindGameObjectsWithTag("column");
   foreach( GameObject c in colums)
      c.GetComponent<MeshRenderer>().materials[1].color =
                                                    newColor.orignalMaterial;
}
于 2013-10-25T08:33:48.380 に答える