4
\$\begingroup\$

I wrote this code, but it feels like it could be a bit more elegant:

Relevant code only:

GameObject newBall;
GameObject pathBall;
if (tag.Equals(pathBallTagName))
{
    newBall = other.gameObject;
    pathBall = gameObject;
}
else
{
    newBall = gameObject;
    pathBall = other.gameObject;
}

Entire .cs file:

using UnityEngine;
using System.Collections;
using BallDelegate;

public class BallCollider : MonoBehaviour {

    public Spline mySpline;
    private SplineController mySplineController;

    public AllBallManager abm;

    public string pathBallTagName = "PathBalls";
    public string ballTagEnd = "Balls";

    public BoxCollider front;
    public BoxCollider back;
    public BoxCollider spawnArea;

    public string frontName = "BallBox Front";
    public string backName = "BallBox Back";

    public bool isInSpawn = true;

    void Start() {
        mySplineController = GetComponent<SplineController> ();
        BoxCollider[] frontBack = GetComponentsInChildren<BoxCollider> ();
        foreach (BoxCollider bc in frontBack)
            if (bc != null) {
                if (bc.name.Equals (frontName))
                    front = bc;
                else if (bc != null && bc.name.Equals (backName))
                    back = bc;
            }

    }

    void OnTriggerEnter(Collider other) {
        if (mySplineController)
        {
            mySplineController.go = true;
            if (mySpline)
                mySplineController.gravSpline = mySplineController.currentSpline = mySpline;
        }
        if (other == spawnArea) {
            isInSpawn = true;
        }
        else if (!isInSpawn) {
            BallDelegateCS bd = other.gameObject.GetComponent<BallDelegateCS>();
            if (bd) // only balls, no other game objects
            {
                GameObject newBall;
                GameObject pathBall;
                if (tag.Equals(pathBallTagName))
                {
                    newBall = other.gameObject;
                    pathBall = gameObject;
                }
                else
                {
                    newBall = gameObject;
                    pathBall = other.gameObject;
                }
                while (newBall.transform.parent)
                    newBall = newBall.transform.parent.gameObject; // make sure we don't get BallBall
                abm.MakeWayFor(newBall, pathBall);
            }
        }
    }

    void OnTriggerExit(Collider other) {
        if (other == spawnArea) {
            isInSpawn = false;
        }

    }
}
\$\endgroup\$
1
  • \$\begingroup\$ The code is not complete and it misses some important elements. To me it looks clean. The only improvement that I can suggest is to use an enum instead of a magic string in if condition and if you are planning to use a string you can specify if this comparison is case insensitive. \$\endgroup\$
    – Sandeep
    Commented May 4, 2014 at 7:04

1 Answer 1

6
\$\begingroup\$

One alternative, note how we must store the bool result to ensure identical semantics (otherwise you risk calling tag.Equals twice... which in addition to performance considerations might change its result between calls or have side effects!)

bool swap = tag.Equals(pathBallTagName);
GameObject newBall = swap ? other.gameObject : gameObject;
GameObject pathBall = swap ? gameObject : other.gameObject;

Alternatively one could implement a Swap function elsewhere:

GameObject newBall = other.gameObject;
GameObject pathBall = gameObject;
if (tag.Equals(pathBallTagName)) Swap (ref newBall, ref pathBall);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.