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;
 }
 }
}
asked May 4, 2014 at 3:49
\$\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\$ 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);
answered May 4, 2014 at 11:32
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.