How to reduce duplicate code

Scripting
1

Ive been developing in C# in unity for a couple years. I get my games to work but struggling over trying to write consise re-usable code. I’ve been reading the “C# programming yellow book” and they say “Block copy is evil”

but how else would I for example write this code using only one if block. in this case I only have sidewalkA and sidewalkB so its not that bad but what if I had C,D,E etc

if (hit2.transform.tag == "SidewalkA") 
{
              for (int i = 0; i < SideWalk_listA.Count; i++)      
             {
                   if (SideWalk_listA[i].transform.position == hit2.transform.position) 
                   {
                        for (int a = 1; a <= i; a++) 
                        {
                            polesA[a].transform.position = SideWalk_listA[a].transform.position;
                            polesA[a].transform.rotation = SideWalk_listA[a].transform.rotation;
                            float sidewalk_length = Mathf.Max(SideWalk_listA[a].GetComponent<Renderer>                  ().bounds.size.x, SideWalk_listA[a].GetComponent<Renderer>().bounds.size.z);
                            if (sidewalk_length < 9f)
                            polesA[a].transform.Rotate(0, -45, 0);
                        }
                        break;
                    }
                }
            }
            else if (hit2.transform.tag == "SidewalkB" ) 
            {
                for (int i = 0; i < SideWalk_listB.Count; i++)     
                {
                    if (SideWalk_listB[i].transform.position == hit2.transform.position)
                    {
                        for (int a = 1; a <= i; a++) 
                        {
                            polesB[a].transform.position = SideWalk_listB[a].transform.position;
                            polesB[a].transform.rotation = SideWalk_listB[a].transform.rotation;
                            float sidewalk_length = Mathf.Max(SideWalk_listB[a].GetComponent<Renderer>().bounds.size.x, SideWalk_listB[a].GetComponent<Renderer>().bounds.size.z);
                            if (sidewalk_length < 9f)
                                polesB[a].transform.Rotate(0, -45, 0);
                        }
                        break;
                    }
                }
            }
2

I would use a switch statement:

switch(hit2.transform.tag)
{
     case "SidewalkA":
      //do stuff
     break;

     case "SidewalkB":
      //do stuff
     break;

     case "SidewalkC":
      //do stuff
     break;

    //and so on...
}
3

Subroutines/functions/methods have been invented for not repeating yourself (DRY). Factor the repeating blocks into a method by passing whatever varies as parameters. Then you can just call that method instead of copy-pasting the block.

Tip: Visual Studio or MonoDevelop have a nice feature that allows you to turn a selected portion of the code into a method.

1 Like
4

When you are copying code - make a note of what you are changing. If you are just changing the variables used (polesA to polesB for example) then that can easily become a function where PolesA, PolesB etc are passed in.
Here’s an example of what it could look like - but obviously…it’s only a guide

void OnCollisionFunction()
{
    if (hit2.transform.tag == "SidewalkA")
    {
        RespondToCollision(hit2, polesA, SideWalk_listA)
    }
    else if (hit2.transform.tag == "SidewalkB")
    {
        RespondToCollision(hit2, polesB, SideWalk_listB)
    }
}

void ResponsdToCollision(GameObject hit2, GameObject[] poles, GameObject[] sideWalk)
{
    for (int i = 0; i < sideWalk.Count; i++)   
    {
        if (sideWalk[a].transform.position == hit2.transform.position)
        {
            for (int a = 1; a <= i; a++)
            {
                poles[a].transform.position = sideWalk[a].transform.position;
                poles[a].transform.rotation = sideWalk[a].transform.rotation;
                float sidewalk_length = Mathf.Max(sideWalk[a].GetComponent<Renderer>().bounds.size.x, sideWalk[a].GetComponent<Renderer>().bounds.size.z);
                if (sidewalk_length < 9f)
                    poles[a].transform.Rotate(0, -45, 0);
            }
            break;
        }
    }
}