0
\$\begingroup\$

I am busy working on a full solo project with no assistance from tutorials or guides. But I have created a monstrosity of an If statement. How could I clean this up and save on memory since I'm calling this in my Update() method and the game is for android?

the hp1 to hp5 are images used as health bars on the hud, I need to deactivate the ones that are not in use when the hp value decreases.

if(hp >= 81)
    {
        hp1.enabled = true;
        hp2.enabled = true;
        hp3.enabled = true;
        hp4.enabled = true;
        hp5.enabled = true;
    }
    else if(hp >= 61)
    {
        hp1.enabled = true;
        hp2.enabled = true;
        hp3.enabled = true;
        hp4.enabled = true;
        hp5.enabled = false;
    }
    else if(hp >= 41)
    {
        hp1.enabled = true;
        hp2.enabled = true;
        hp3.enabled = true;
        hp4.enabled = false;
        hp5.enabled = false;
    }
    else if (hp >= 21)
    {
        hp1.enabled = true;
        hp2.enabled = true;
        hp3.enabled = false;
        hp4.enabled = false;
        hp5.enabled = false;
    }
    else if (hp >= 1)
    {
        hp1.enabled = true;
        hp2.enabled = false;
        hp3.enabled = false;
        hp4.enabled = false;
        hp5.enabled = false;
    }
    else if(hp < 1)
    {
        hp1.enabled = false;
        hp2.enabled = false;
        hp3.enabled = false;
        hp4.enabled = false;
        hp5.enabled = false;
    }
\$\endgroup\$
4
  • \$\begingroup\$ I'm not sure what you mean by "save on memory" - if you have 5 booleans you have 5 booleans. You might be able to change this to using a bitmask instead, but that would be a complete rewrite of what you have right now. \$\endgroup\$
    – UnholySheep
    Commented Dec 28, 2016 at 21:18
  • \$\begingroup\$ As for simplifying this, you could for example set all values to true at the start of Update and then set them to false individually when a hp is below the threshold (e.g.: if(hp < 81) hp5.enabled = false;). This way you'd replace the if- else chain with just 5 if statements. Although I am not sure why you need to have this in Update in the first place, as you should be able to change those values in the same place where you modify hp? \$\endgroup\$
    – UnholySheep
    Commented Dec 28, 2016 at 21:21
  • 1
    \$\begingroup\$ I'm afraid this is actually off topic, here. Your using your code in game dev, but what your asking is not specific to game dev. In fact, if statements and the concept of cleaning up these if statements are as generic as they come. The best way to look at it is to ask yourself if a game developer or a programmer would better answer your question; and if your asking the same thing, when you remove all game-based aspects. Note that for what your asking,I believe CodeReview is the right place, where the close reason suggests StackOverflow. \$\endgroup\$
    – Gnemlock
    Commented Dec 28, 2016 at 22:44
  • \$\begingroup\$ In case pointed out that hp.enabled = true is specific, I would argue that the core component of the question asks about setting bools. The fact that this bool is based in UnityEngine has no effect on the question or answer. \$\endgroup\$
    – Gnemlock
    Commented Dec 28, 2016 at 22:49

3 Answers 3

Reset to default
2
\$\begingroup\$

Making it more efficient (saving CPU cycles) will be a matter of making sure you only call this chunk of code when hp changes. That being said, you can easily simplify this to five lines of code:

hp1.enabled = hp >= 1;
hp2.enabled = hp >= 21;
hp3.enabled = hp >= 41;
hp4.enabled = hp >= 61;
hp5.enabled = hp >= 81;

If you want to make it easier to maintain, you can put your hp bars in an array and loop over them (see the pseudocode below):

int healthInterval = 20;
for(int i = 0; i < bars.length; i++) {
    bars[i].enabled = hp >= 1 + healthInterval * i;
}

[Edit]: I suppose the latter might take up more memory, but it will be negligible, and this method is the first step towards having dynamic health bars which you can give different numbers of bars and different amounts of HP per bar without re-writing your code.

\$\endgroup\$
4
  • \$\begingroup\$ What about the second suggestion could take up more memory? \$\endgroup\$
    – UnholySheep
    Commented Dec 28, 2016 at 21:56
  • \$\begingroup\$ The local variable healthInterval. If the compiler may be smart enough to realize that it is a constant and deal with that though. Either way we're talking a difference of a few bytes of memory which should be negligible. \$\endgroup\$
    – John
    Commented Dec 28, 2016 at 21:59
  • \$\begingroup\$ I think the compiler will easily optimize it away, especially if you add const to the declaration. Also I believe OP was more "worried" about more persistent memory usage (e.g.: class members) rather than a bit of extra memory in function calls \$\endgroup\$
    – UnholySheep
    Commented Dec 28, 2016 at 22:03
  • \$\begingroup\$ I love this answer! I find that often times logical condition assignment is forgotten and if-statements with assognment inside are used instead. \$\endgroup\$
    – user35344
    Commented Dec 28, 2016 at 22:22
0
\$\begingroup\$

Instead of setting each booleans' value in the if statements, you should set all of them to true, then only change the booleans, which need to be false.

hp1.enabled = true;
hp2.enabled = true;
hp3.enabled = true;
hp4.enabled = true;
hp5.enabled = true;

if(hp >= 61)
{
    hp5.enabled = false;
}
else if(hp >= 41)
{
    hp4.enabled = false;
    hp5.enabled = false;
}
else if (hp >= 21)
{
    hp3.enabled = false;
    hp4.enabled = false;
    hp5.enabled = false;
}
else if (hp >= 1)
{
    hp2.enabled = false;
    hp3.enabled = false;
    hp4.enabled = false;
    hp5.enabled = false;
}
else if(hp < 1)
{
    hp1.enabled = false;
    hp2.enabled = false;
    hp3.enabled = false;
    hp4.enabled = false;
    hp5.enabled = false;
}

Second: Currently, hp5.enabled is false, when hp is either

  • smaller than 1
  • more than or equal to 1
  • more than or equal to 21
  • more than or equal to 41
  • more than or equal to 61

So it is pretty much when hp is smaller than 81

hp4.enabled is false, when hp is

  • smaller than 1
  • more than or equal to 1
  • more than or equal to 21
  • more than or equal to 41

or in short, when hp is less than 61. You can do this to every boolean, and you get the following

hp1.enabled = true;
hp2.enabled = true;
hp3.enabled = true;
hp4.enabled = true;
hp5.enabled = true;

if (hp < 1) {
    hp1.enabled = false;
}

if (hp < 21) {
    hp2.enabled = false;
}

if (hp < 41) {
    hp3.enabled = false;
}

if (hp < 61) {
    hp4.enabled = false;
}

if (hp < 81) {
    hp5.enabled = false;
}
\$\endgroup\$
3
  • \$\begingroup\$ "hp2.enabled will always be false, because hp is always either smaller, bigger than or equal to 1"?? I have no idea where that comes from - that is plainly not what is happening in OP's original code \$\endgroup\$
    – UnholySheep
    Commented Dec 28, 2016 at 21:32
  • \$\begingroup\$ @UnholySheep Messed up something, let me fix it real quick \$\endgroup\$
    – Bálint
    Commented Dec 28, 2016 at 21:35
  • \$\begingroup\$ @UnholySheep fixed \$\endgroup\$
    – Bálint
    Commented Dec 28, 2016 at 21:45
-3
\$\begingroup\$

Why don't you create an two-dimensional array that stores the health it needs to be to be on(index 0) and the hp object itself(index 1). Then you can write code something like this:

C# is one of the few languages I dont fully know, but in basic programming syntax...

foreach(miniArray in biggerArray){
if(hp >= miniArray[0]){
miniArray[0].enable = true;
}
else{
miniArray[0].enable = false;
}
}

You will have to change this code up a bit because its not actually c#, but basic logic is correct.

\$\endgroup\$
7
  • \$\begingroup\$ LMAO as I was typing the guy above me made a better description of my same idea. Well no need for this answer i guess \$\endgroup\$
    – Quinn Guerin
    Commented Dec 28, 2016 at 21:48
  • 1
    \$\begingroup\$ This answer is also plain wrong. It would take up more memory and you have errors in your example code. Also there would be better options than 2-dimensional arrays to do this exact thing you are proposing. And finally saying "c# makes things weird" is not a going to make you many friends. \$\endgroup\$
    – UnholySheep
    Commented Dec 28, 2016 at 21:52
  • \$\begingroup\$ My answer was for making the code easy to read not making it faster. Where r the errors? \$\endgroup\$
    – Quinn Guerin
    Commented Dec 28, 2016 at 22:01
  • 1
    \$\begingroup\$ It doesn't make the code easier to read either as it relies on magic numbers in a two dimensional array. And you try to enable the wrong one in the else clause. (Which kinda proves that it is not easier to read) \$\endgroup\$
    – UnholySheep
    Commented Dec 28, 2016 at 22:04
  • \$\begingroup\$ Fixed ;) I still maintain 10 lines is easier to read then one thousand lines. \$\endgroup\$
    – Quinn Guerin
    Commented Dec 28, 2016 at 22:13

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .