SpoocyCrep
SpoocyCrep

Reputation: 614

How can I write it cleaner?

I know, this is probably a very silly question.

I have a long code that repeats itself and uses a lot of copy+paste, I know this is a very bad practice to copy paste the same code but I can't figure out a way to short it! The code I'm struggling to write cleaner is a code that checks around a vector other vectors. I do it by changing the x by adding +1 and -1, then changing the y +1 and -1 and at the end z+1 and z-1, for each slight change I call the same function for the slightly changed vector. This creates a really ugly code:

 void checkAirRadius(Voxel temp, Vector3 fireposition)
{
    Vector3 original = fireposition;
    if (temp.type != null)
    {
        if (temp.type.isFire != true && temp.type.burnable == true)
        {
            fireposition.x -= 1;
            Voxel temp2 = env.GetVoxel(fireposition);
            if (temp2.type == null)
                if (temp2.hasContent != 1)
                {
                    env.VoxelDestroy(original);
                    env.VoxelPlace(fireposition, firevoxel);
                    coroutine = WaitAndPrint(1.0f, fireposition, 1);
                    StartCoroutine(coroutine);

                }
            fireposition.x += 1;
            fireposition.x += 1;
            temp2 = env.GetVoxel(fireposition);
            if (temp2.type == null)
                if (temp2.hasContent != 1)
                {
                    env.VoxelDestroy(original);
                    env.VoxelPlace(fireposition, firevoxel);
                    coroutine = WaitAndPrint(1.0f, fireposition, 1);
                    StartCoroutine(coroutine);

                }
            fireposition.x -= 1;
            fireposition.y += 1;
            temp2 = env.GetVoxel(fireposition);
            if (temp2.type == null)
                if (temp2.hasContent != 1)
                {
                    env.VoxelDestroy(original);
                    env.VoxelPlace(fireposition, firevoxel);
                    coroutine = WaitAndPrint(1.0f, fireposition, 0);
                    StartCoroutine(coroutine);

                }
            fireposition.y -= 1;
            fireposition.y -= 1;
            temp2 = env.GetVoxel(fireposition);
            if (temp2.type == null)
                if (temp2.hasContent != 1)
                {
                    env.VoxelDestroy(original);
                    env.VoxelPlace(fireposition, firevoxel);
                    coroutine = WaitAndPrint(1.0f, fireposition, 2);
                    StartCoroutine(coroutine);

                }
            fireposition.y += 1;
            fireposition.z -= 1;
            temp2 = env.GetVoxel(fireposition);
            if (temp2.type == null)
                if (temp2.hasContent != 1)
                {
                    env.VoxelDestroy(original);
                    env.VoxelPlace(fireposition, firevoxel);
                    coroutine = WaitAndPrint(1.0f, fireposition, 1);
                    StartCoroutine(coroutine);

                }
            fireposition.z += 1;
            fireposition.z += 1;
            temp2 = env.GetVoxel(fireposition);
            if (temp2.type == null)
                if (temp2.hasContent != 1)
                {
                    env.VoxelDestroy(original);
                    env.VoxelPlace(fireposition, firevoxel);
                    coroutine = WaitAndPrint(1.0f, fireposition, 1);
                    StartCoroutine(coroutine);

                }
        }
    }
}
void FireUpdate()
{
    for (int i = poolFire.Count - 1; i >= 0; i--)
    {
        if (Random.Range(1, 10) == 5 || VoxelPlayEnvironment.instance.GetVoxel(new Vector3(poolFire[i].x, poolFire[i].y - 1, poolFire[i].z)).type == null)
        {
            poolFire.RemoveAt(i);
            return;
        }
        else
        {

            Vector3 fireposition = poolFire[i];
            fireposition.x -= 1;
            Voxel temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
            checkAirRadius(temp, fireposition);
            fireposition.x += 1;
            fireposition.x += 1;
             temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
            checkAirRadius(temp, fireposition);
            fireposition.x -= 1;
            fireposition.y += 1;
             temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
            checkAirRadius(temp, fireposition);
            fireposition.y -= 1;
             temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
            checkAirRadius(temp, fireposition);
            fireposition.y -= 1;
             temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
            checkAirRadius(temp, fireposition);
            fireposition.z += 1;
             temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
            checkAirRadius(temp, fireposition);
            fireposition.z -= 2;
             temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
            checkAirRadius(temp, fireposition);



        }
    }

I wonder if there is a simple function I can create that will take one vector and call functions for around that block without having to manually change x, y,z by 2 points every time.

Upvotes: 0

Views: 96

Answers (1)

Vitor Figueredo
Vitor Figueredo

Reputation: 629

Following to the DRY (don't repeat yourself) principle, every time you see a code that's being reused, you can, and should, refactor it.

As for your code, the first thing I'd do is create a function that receives the new fireposition. Let's say for example:

void ChangeFirePositionAndRecalculate (int newValue) {
    fireposition.z += newValue;
    temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
    checkAirRadius(temp, fireposition);
}

Instead of calling:

fireposition.z -= 2;
temp = VoxelPlayEnvironment.instance.GetVoxel(fireposition);
checkAirRadius(temp, fireposition);

You can call:

ChangeFirePositionAndRecalculate (-2);
ChangeFirePositionAndRecalculate (1);

But, then again, DRY. You can, for example, create an array, with all these positions:

public int [] positions;

This will even allow you to change the fireposition directly from the editor. And with that in mind, you can now run:

for (int i = 0; i < position.lenght; i++) {
    ChangeFirePositionAndRecalculate (positions [i]);
}

And you can figure the rest. This is just some things I'd do.

[EDIT] I forgot to mention that when creating methods, use CamelCase, I've noticed that some of your methods use but checkAirRadius doesn't.

Upvotes: 4

Related Questions