koobas.hobune.stream
How can I avoid using switch and for too much?
Hello, From time to time I've found myself in this position:
switch (parentClicked.name){
case "Damage":
for (int i = 0; i < availableWeapons.Count; i++) {
if (weaponSpriteName == availableWeapons [i].weaponName)
calculatedValue = CalculateUpgrade ( availableWeapons [i].damage, availableWeapons [i].damageLevel);
}
break;
case "Accuracy":
for (int i = 0; i < availableWeapons.Count; i++) {
if (weaponSpriteName == availableWeapons [i].weaponName)
calculatedValue = CalculateUpgrade ( availableWeapons [i].accuracy, availableWeapons [i].accuracyLevel);
}
break;
case "AmmoQuantity":
for (int i = 0; i < availableWeapons.Count; i++) {
if (weaponSpriteName == availableWeapons [i].weaponName)
calculatedValue = CalculateUpgrade ( availableWeapons [i].ammoQuantity, availableWeapons [i].ammoLevel);
}
break;
case "RateOfFire":
for (int i = 0; i < availableWeapons.Count; i++) {
if (weaponSpriteName == availableWeapons [i].weaponName)
calculatedValue = CalculateUpgrade ( availableWeapons [i].rateOfFire, availableWeapons [i].rateOfFireLevel);
}
break;
}
Basically using the switch statement with the for state too much. And this is just an example. That for statement in the switch statement is much bigger sometimes.
But, i can't, or i don't know a way around to only use a function like this:
public void TestFunction (float upgradeParameter, float upgradeLevel){
for (int i = 0; i < availableWeapons.Count; i++) {
if (weaponSpriteName == availableWeapons [i].weaponName)
calculatedValue = CalculateUpgrade ( availableWeapons [i].upgradeParameter, availableWeapons [i].upgradeLevel);
}
}
And in the switch statement i would pass the parameters to the TestFuction (float upgradeParameter, float upgradeLevel).
But this doesn't work because availableWeapons [i].upgradeParameter and availableWeapons [i].upgradeLevelobviously don't exist in the availableWeapons[].
Can anyone please enlighten me on this simple matter of avoiding too much switches or too much fors?
Hi @Subject-Gabz,
The for-loop can be simplified by doing it once in advance to get the target weapon. (I'm assuming you're always targeting one weapon at a time).
That way you would end up with this:
// ..........
public void Example()
{
ExampleWeapon targetWeapon = null;
for( int i = 0; i < availableWeapons.Count; i++ )
{
if( weaponSpriteName == availableWeapons[ i ].weaponName )
{
targetWeapon = availableWeapons[ i ];
}
}
if( targetWeapon != null )
{
switch( parentClicked.name )
{
case "Damage":
calculatedValue = CalculateUpgrade( targetWeapon.damage, targetWeapon.damageLevel );
break;
case "Accuracy":
calculatedValue = CalculateUpgrade( targetWeapon.accuracy, targetWeapon.accuracyLevel );
break;
case "AmmoQuantity":
calculatedValue = CalculateUpgrade( targetWeapon.ammoQuantity, targetWeapon.ammoLevel );
break;
case "RateOfFire":
calculatedValue = CalculateUpgrade( targetWeapon.rateOfFire, targetWeapon.rateOfFireLevel );
break;
}
}
}
// ..........
Personally, since you're using WeaponStat.value + WeaponStat.level variables a lot, I would refactor it into this, by adding a WeaponStat class to your Weapon class. This WeaponStat would have the CalculateUpgrade method built-in:
using UnityEngine;
using System.Collections;
using System.Collections.Generic;
public class Simplify : MonoBehaviour
{
private List<ExampleWeapon> availableWeapons = new List<ExampleWeapon>();
private string weaponSpriteName;
private Transform parentClicked;
private float calculatedValue;
public void Example()
{
ExampleWeapon targetWeapon = null;
// Get target weapon.
for( int i = 0; i < availableWeapons.Count; i++ )
{
if( weaponSpriteName == availableWeapons[ i ].weaponName )
{
targetWeapon = availableWeapons[ i ];
}
}
// Only proceed if a weapon was found.
if ( targetWeapon != null )
{
// Calculate upgrade.
switch( parentClicked.name )
{
case "Damage":
calculatedValue = targetWeapon.damage.CalculateUpgrade();
break;
case "Accuracy":
calculatedValue = targetWeapon.accuracy.CalculateUpgrade();
break;
case "AmmoQuantity":
calculatedValue = targetWeapon.ammoQuantity.CalculateUpgrade();
break;
case "RateOfFire":
calculatedValue = targetWeapon.rateOfFire.CalculateUpgrade();
break;
}
}
}
}
// Applied your code styling here. (normally I'd use Damage or m_damage)
public class ExampleWeapon
{
public string weaponName;
public WeaponStat damage;
public WeaponStat accuracy;
public WeaponStat ammoQuantity;
public WeaponStat rateOfFire;
}
public class WeaponStat
{
public float value;
public float level;
public float CalculateUpgrade()
{
// Just an example.
return value * (level + 1);
}
}
You could then add / edit variables to your needs whilst keeping it more compact. You're also detecting the variable you wish to edit via Parent Name. There's so more gain to be had there, but that's a bit more complicated, haha.
I hope that helps, if it did, please accept this answer, it'd be much appreciated! If you need any more details, let me know! :)
Best of luck!
Cheers,
ThePersister
@doublemax Answer solved the first part of your suggestion in a more elegant way. But, you also made me realise that I could create a class for WeaponStat and create a function to calculate the upgrade.
I created the WeaponStat class and implemented as far as i could on the rest of my game, but couldn't save those variables in my database SQlite4Unity because WeaponClass had variable such as : public WeaponStat damage; public WeaponStat accuracy; public WeaponStat ammoQuantity; public WeaponStat rateOfFire;
And, unfortunately, couldn't figure out a way to inject "a class inside a class" to my database (which also needs to be improved, the database). But I created a method to calculate the upgrade in WeaponClass and saved me a few more lines of code.
And yes, relying on strings to find things is really a bad habit, sometimes i can avoid it, others is just a quick way to proceed in developing/testing stuff. But as soon as I've all the "sections" of my game solved, I'll proceed to refine the performance and flexibility of the code.
Thanks for your help.
Ah right, you're welcome! :)
You could add [System.Serializable] to the WeaponStat class, to have it serialize better, that might help with database / serilization issues. Or at least it will help you edit values in the inspector, sorry I skipped that, same goes for the Weapon class:
[System.Serializable]
public class ExampleWeapon
{
public string weaponName;
public WeaponStat damage;
public WeaponStat accuracy;
public WeaponStat ammoQuantity;
public WeaponStat rateOfFire;
}
[System.Serializable]
public class WeaponStat
{
public float value;
public float level;
public float CalculateUpgrade()
{
// Just an example.
return value * (level + 1);
}
}
I hope that helps too!
Thanks for your kind replies, if that's all for this question, please consider accepting my answer, it'd mean a lot to me :)
Best of luck!
Cheers,
ThePersister
I still get a NotSupportedException:
NotSupportedException: Don't know about SQLite.models.WeaponStat SQLite.Orm.SqlType
Any idea? This is way beyond the initial question so I'll accept your answer anyway. But if you know anything about this problem i would be glad to know.
Thanks!
In a first step you could simplify it like this:
float upgradeParameter = 0.0f;
float upgradeLevel = 0.0f;
Weapon weapon = FindWeapon( parentClicked.name );
if( weapon )
{
switch (parentClicked.name)
{
case "Damage":
upgradeParameter = weapon.damage;
upgradeLevel = weapon.damageLevel;
break;
case "Accuracy":
upgradeParameter = weapon.accuracy;
upgradeLevel = weapon.accuracyLevel;
break;
case "AmmoQuantity":
upgradeParameter = weapon.ammoQuantity;
upgradeLevel = weapon.ammoLevel;
break;
case "RateOfFire":
upgradeParameter = weapon.rateOfFire;
upgradeLevel = weaponrateOfFireLevel;
break;
}
calculatedValue = upgradeParameter ( value, upgradeLevel );
}
// ****
Weapon FindWeapon( string name )
{
for (int i = 0; i < availableWeapons.Count; i++)
{
if (name == availableWeapons [i].weaponName)
return availableWeapons [i];
}
return null;
}
If that same switch would occur more than once, you could move that into weapon class.
Thanks a lot for remembering me that some functions should be implemented in the WeaponClass, VehicleClass, AmmoClass, etc. Your answer helped me realise how sometimes I was approaching the creation of some functions, not in the most efficient way. And therefore creating a more light weight switch and for statement. Thanks.
