How would you shorten that piece of code?
Just a quick question. I have a piece of Code and i dont know how i can shorten that better. How would you compress that?
IEnumerator ActivatePickUp()
{
if (Random.Range(0,2) == 0)
{
if (shotAmount < 5)
{
shotAmount ++;
yield return new WaitForSeconds(PickUpTime);
shotAmount --;
}
else
{
if (fireRate > 0.075)
{
fireRate /= 2;
yield return new WaitForSeconds(PickUpTime);
fireRate *= 2;
}
}
}
else
{
if (fireRate > 0.075)
{
fireRate /= 2;
yield return new WaitForSeconds(PickUpTime);
fireRate *= 2;
}
else
{
if (shotAmount < 5)
{
shotAmount ++;
yield return new WaitForSeconds(PickUpTime);
shotAmount --;
}
}
}
}
Answer by troien · Sep 30, 2016 at 11:17 AM
Well, as in literal lines of code, you could use "else if" at a few places..
Like so:
IEnumerator ActivatePickUp()
{
if (Random.Range(0,2) == 0)
{
if (shotAmount < 5)
{
shotAmount ++;
yield return new WaitForSeconds(PickUpTime);
shotAmount --;
}
else if (fireRate > 0.075)
{
fireRate /= 2;
yield return new WaitForSeconds(PickUpTime);
fireRate *= 2;
}
}
else if (fireRate > 0.075)
{
fireRate /= 2;
yield return new WaitForSeconds(PickUpTime);
fireRate *= 2;
}
else if (shotAmount < 5)
{
shotAmount ++;
yield return new WaitForSeconds(PickUpTime);
shotAmount --;
}
}
Normally you could then combine duplicate code into seperate methods and call these methods instead of continuously using the duplicate code. But considering you have just a limited piece of duplicate code and because yield return is a part of your duplicate code (which is annoying as it would mean that any seperate method would require to be a ienumerator aswell). I don't really see any benefits of doing that as in this use case it would really just make the code less readable...
Answer by Bunny83 · Sep 30, 2016 at 11:29 AM
You mean something like this?
IEnumerator ActivatePickUp()
{
bool c1 = (fireRate > 0.075);
if (((Random.Range(0,2) == 0) || !c1)&& (shotAmount < 5))
{
shotAmount ++;
yield return new WaitForSeconds(PickUpTime);
shotAmount --;
}
else if (c1)
{
fireRate /= 2;
yield return new WaitForSeconds(PickUpTime);
fireRate *= 2;
}
}
This doesn't handle the case when Random.Range(0,2) == 1 && fireRate <= 0.075 && shotAmount < 5
(the very bottom case in the OP's code)
I've realised this the moment i posted my answer. I've already changed my answer about 3 $$anonymous$$utes before your comment ^^.
This is pretty much the "shortest" way. However i'm not sure if that has any advantages. It makes it harder to read / understand and it doesn't improve anything neither performance nor code complexity.
Answer by MrEastwood · Sep 30, 2016 at 03:12 PM
This ended up actually being longer, but the following piece of code has no duplication of logic, that is you have one place where you can change the rules for each condition.
void ActivatePickUp()
{
if (Random.Range(0,2) == 0)
{
if (!tryShotAmount()) tryFireRate();
}
else
{
if (!tryFireRate()) tryShotAmount();
}
}
bool tryShotAmount()
{
if (shotAmount < 5)
{
StartCoroutine(ActivateShotAmount());
return true;
}
return false;
}
bool tryFireRate()
{
if (fireRate > 0.075)
{
StartCoroutine(ActivateFireRate());
return true;
}
return false;
}
IEnumerator ActivateShotAmount()
{
shotAmount ++;
yield return new WaitForSeconds(PickUpTime);
shotAmount --;
}
IEnumerator ActivateFireRate()
{
fireRate /= 2;
yield return new WaitForSeconds(PickUpTime);
fireRate *= 2;
}