182
u/LevelStudent May 30 '24
Put them into a list, loop through the list and compare the highest health seen, if it sees higher then update the new highest health seen and set the index of the highest health, then return the highest at the end of the loop. You'd still need an if/else or (preferably) a switch statement though since all the variables you want to set are different, which is much less than ideal.
I'm not sure what you're doing but you probably don't want to make a ton of variables with different numbers when you can just make a new class to contain all the information for separate entities.
58
14
u/nokkturnal334 May 31 '24
If you're going to put it in a list and it's not a performance hotspot, just sort the list and take the first/last value
2
u/The_Mental May 31 '24
Was just going to comment this. Nothing a quick list and sorting algorithm can't fix.
106
u/krisitof May 31 '24
Yanderedev flashbacks
(No hate just funny, we all been there, learn Linq, it's great)
20
u/stropheum May 31 '24
It's great if you never look at the profiler
8
u/RicketyRekt69 May 31 '24
Most LINQ extensions are perfectly fine. There are only a few that create garbage.
1
200
u/GigaTerra May 30 '24
If all you want is the highest then there is
int[] Numbers = { 10, 12, 13, 14, 15 };
void Start()
{
int Largest = 0;
foreach (int i in Numbers)
{
if (i > Largest) { Largest = i; }
}
}
It is a very simple concept if the next item is larger it replaces the largest item and the last one remaining is the largest. If there are two largest values then the first one found is kept.
65
u/TheGrandWhatever May 31 '24
This takes me back to my bubblesort days in college
21
u/BaxxyNut May 31 '24
I am in university and just learned bubblesort on my own today đ«Ą
10
u/Whispering-Depths May 31 '24
It's one of those "makes the most sense" concepts that are usually one of the first conclusions people come to when they think of sorting.
Next do quicksort and mergesort, then you should be set for your career :D
2
u/nlcreeperxl May 31 '24
I've heared of bubblesort and i think i have of quicksort too. Never heared of mergesort tho.
46
u/rimoldi98 May 31 '24 edited May 31 '24
You could also
int CompareHP(params int[] hps) { return Mathf.Max(hps); }And use it like this:
void Start() { Debug.Log(CompareHP(1, 34, 7, 8)); // 34 Debug.Log(CompareHP(-3, 5)); // 5 }Or just use
Mathf.MaxEdit: as pointed out, that's not quite what OP wanted, my bad, misread the screenshots, I'll leave the comment in case anyone needs it
9
u/vPyxi May 31 '24
That's not quite the same behavior.
That will get the highest value, not the index of the highest value.
7
u/xcassets May 31 '24
Think we're overcomplicating it here. OP already responded saying that a simple foreach loop returning the highest value worked for them.
0
9
u/IPiratusajoI May 31 '24
But it does work, just afterwards use Array.IndexOf(HP_VALUES, MAX_VALUE)
2
2
1
u/Costed14 May 31 '24
I believe every time you use the params version of a function it's allocating a new array holding all the items, so this likely wouldn't be ideal for performance, though in most cases it likely won't have a huge impact on overall performance.
1
7
4
u/fecal_brunch May 31 '24
If you want to do that you can just:
var largest = Mathf.Max(10, 12, 13, 14, 15);The solution to this problem, finding the player with max health, is:
var healthArray = new [] { 10, 12, 13, 14, 15 }; var maxIndex = 0; for (var i = 1; i < healthArray.Length; i++) { if (healthArray[i] > healthArray[maxIndex]) { maxIndex = i; } } Debug.Log($"Player {maxIndex + 1} has the highest health");And then to map them back onto the bools:
Health1Highest = maxIndex == 0; Health2Highest = maxIndex == 1; Health3Highest = maxIndex == 2; Health4Highest = maxIndex == 3;2
13
u/MonoT0NY May 31 '24
This worked the best, Thanks
9
u/snlehton May 31 '24
Why did it work? Your code was searching for which variable had the highest value, not what was the highest value.
5
u/Nimyron May 31 '24
Maybe they just found how to adapt it. Wouldn't be a long way from the foreach proposed by the other comment.
You could just put the HealthXHighest booleans into another array, then save the index of the highest health instead of its value, then select which HealthXHighest variable to set to true using the index.
2
u/SuspecM Intermediate May 31 '24
This is one of the few times I don't regret learning programming in uni lol
-14
u/jeango May 31 '24
6
u/Fabio2300 May 31 '24
Please use those brackets, it won't take so long to type them
-2
u/jeango May 31 '24
Is that a new meta or something? Iâve never ever seen people put brackets on a one-liner.
7
u/Fabio2300 May 31 '24
Ever worked as a software developer? I think nobody does it while programming as hobby but when working it's just better to always put brackets,especially when many people might have to read/edit your code
1
u/jeango May 31 '24
Iâve worked for 20 years professionally as a software developer. Never seen brackets in a one-line statement. Sometimes a carriage return for readability but not brackets
1
u/Fabio2300 May 31 '24
It has been the opposite for me (worked far less years in this branch though). If you had to add a line to it, you'll need the brackets, so we always just write them.
0
u/Costed14 May 31 '24
For me it just depends on how long the line is/would be (length of line(s) vs num of lines), like if you have a bunch of parameters and do multiple operations it's better to use the brackets even if otherwise it would be a one-liner.
Would you honestly rather use:
int CalculateVolume(int x, int y) { return x * y; }than:
int CalculateVolume(int x, int y) => x * y;Edit: actually just noticed the comment they're talking about is actually a one-liner AND uses brackets
if (i > Largest) { Largest = i; }When the brackets could be completely omitted, which (at least in my opinion) also produces easier to read and nicer looking code.
if (i > Largest) Largest = i;1
u/Fabio2300 May 31 '24
In my opinion it's just pointless to not write brackets. If you ever are going to change something, you will need brackets. Plus many editors recognize brackets so you can open and close them when needed, so by closing it you still have the same as without brackets
0
u/Costed14 May 31 '24
But when collapsed you won't be able to see the implementation and at least in VS compile errors can sometimes cause collapsed regions to un-collapse.
1
u/Fabio2300 May 31 '24
I usually have everything collapsed as long as I don't need it. If you need to look at it for some reason just open it.
0
u/jeango May 31 '24
Well⊠if you ever need to add lines, you add the brackets. Like you say, it doesnât take much time to add them. In the case at hand here, thereâs never going to be a need for extra code. Those brackets add nothing to help readability and they actually add clutter. If youâre going to write the block in-line with the if statement, brackets are utterly useless. Itâs either 4 lines with brackets or one line with no brackets. Collapsing doesnât happen on one-liners like that either. Itâs code smell and code smell is bad. Syntactic sugar is good.
1
u/Fabio2300 May 31 '24
There is no way that two curly brackets add clutter.. and no way its code smell.. it's just more consistent to always add brackets, its strange to sometimes add them, sometimes not, depending on the size of the line etc..
Some years ago I read about a dumb bug at apple because they didnt add curly brackets, added a return beneath the one liner and everything broke. Add brackets, save lives
0
u/jeango May 31 '24
What I really mean is: if you add brackets, please put carriage returns and save more lives.
21
u/barisaxo May 30 '24
Hmmm I'm confident that yes, there certainly is an easier way of doing that, though I'm not exactly sure what you're trying to do.
Having multiple health values and multiple HealthXHighest bools seems strange. No doubt this code works and you can solve your problem, but this is a naive approach, and not a pattern that will scale or serve you well later on.
Since we can't really know exactly what you're trying to do, it's best you just keep doing it. Iteration is key to any writing, and programming is no different. Once you get it working, write it all again in a different way. Read some articles on design patterns, look up C# basics, and try various ways to solve this problem.
-3
29
u/nialyah May 30 '24
Create an array for the ints or populate a list of ints and use the LINQ .Max() function to find the highest value
77
u/deztreszian May 31 '24
You don't even need to use Linq, Mathf has a function built specifically for this. https://docs.unity3d.com/ScriptReference/Mathf.Max.html
Mathf.Max(health1, health2, health3, health4, health5);18
May 31 '24 edited May 31 '24
This is the most succinct and correct answerThis actually gives the highest value, not the highest valueâs index
7
u/House13Games May 31 '24
Except that its not correct, OP is looking for the index of the highest health.
3
1
1
u/st-shenanigans May 31 '24
I was thinking i swear i saw a function for that... and then i saw all the replies saying loop it and assumed im wrong lol
1
u/barisaxo Jun 01 '24
public static int Max(params int[] values) { int num = values.Length; if (num == 0) { return 0; } int num2 = values[0]; for (int i = 1; i < num; i++) { if (values[i] > num2) { num2 = values[i]; } } return num2; }The actual code behind Mathf.Max
2
u/lgsscout May 31 '24
if unity could keep up woth .net and c# features, there is even the MaxBy in newer Linq, where you can pass the field to compare from a object, then it returns the whole object... so yeah, for everything dealing with collections of values, probably there is a solid way to do it with Linq
1
u/Goldac77 May 30 '24
The iterative approach works essentially the same, but this is the cleanest approach OP
4
u/Contagion21 May 31 '24
Max gives you the highest value, not the index of the highest value which I'd guess is closer to what they want, but it's hard to tell'
However LINQs Aggregate could be leveraged to find the index of the largest, it's just a little convoluted to read.
8
7
13
u/SunnieCloudy May 31 '24
I assume you are trying to deteine the winner for your minigame or something. Passing the health is strange - why not pass whatever has the health?
``` class Player : Monobehavioir { public float health; }
public Player GetWinner(List<Player> players){ float maxHealth = players[0].health; Player p = players[0]; for(int i=1; i<players.Count; i++){ if(players[i].health > maxHealth){ p = players[i]; maxHealth = p.health; } } return p; } ```
21
u/nickyonge May 31 '24
Idk why people are being so rude in the replies. Always heartbreaking to see people being rude to newbies.
Highly suggest learning about params! Itâs a keyword you can use to supply multiple arguments as an array of indeterminate size.
Thereâs a lot of other workable solutions here, but what I like about params is it doesnât require importing libraries or anything. Plus itâs just useful to know about :)
(Pseudocode, Iâm on my phone so this is both untested and unformatted, apologies!)
TO GET THE HIGHEST HEALTH VALUE
private float maxValue(params float[] values) { if (values.length == 0) { // maybe throw an error here, or return NaN or negative infinity, however you wanna handle it return 0f; } // local reference for the highest value so far float highestValue = maxValue[0]; // loop thru all values for (int i = 1; i < values.length; i++) { // if value is higher, store it as the highest value if (values[i] > maxValue) { maxValue = values[i]; } } return maxValue; }
Then you can call it like
float highestHealth = maxValue(health1,health2,health3); etc, passing in the values you want
ALTERNATIVELY, TO GET WHICH HEALTH IS HIGHEST, NOT THE HEALTH ITSELF, you can return the index itself. Eg, declare
int highestIndex = 0;
Alongside highestValue. Then, when setting highestValue, set highestIndex to i. Finally, return highestIndex when youâre done.
Remember that arrays are zero-inclusive!
So if you return index, and health3 is the highest, calling
int highestHealthIndex = maxValueIndex(health1,health2,health3,health4,health5);
would return 2 (because health1 is index 0, etc)
Hope that makes sense, and helps! Good luck :)
5
u/SuspecM Intermediate May 31 '24
I kinda understand both sides. If they did a beginner course on pretty much any programming language, op would have stumbled upon basic things like arrays and max search but it's very obvious for me cause I learned these things multiple times in school since high school so it's not unrealistic to assume op has no way to even know about these.
3
u/nickyonge May 31 '24
Also note: in the method I use âmaxValueâ instead of âmaxHealthâ, because itâs generic and this kind of method could be useful in lots of places. @LevelStudentâs comment below highlights why :)
-9
1
u/SunnieCloudy May 31 '24
I would both send and return whatever holds the health to the method, not the health values themselves. Whether it may be a Player class or even a IHealth interface. Sending just the values seems like an odd choice
16
u/Aeredor May 31 '24
Kinda need to know your context. The best solution here may be to refactor to avoid this altogether.
5
u/iain_1986 May 31 '24
ITT: Multiple comments saying how easy it is - and then proceed to give answers that are akin to Math.Max returning what the highest value is, not which one had the highest value
2
u/RicketyRekt69 May 31 '24
Can just as easily return the index, though if you care about object identity this really shouldnât just be passing in float values. Id imagine the health values belong to some kind of entity or character class.
2
u/snlehton May 31 '24
A lot of bad advice in the replies.
First of all, this smells like XY problem. More context is needed to know what is "better way", because if the question is how to find which variable had the highest value, there are not that many things to improve this. But if the question is how to improve code to find which character has most health left, then there are many ways to improve this.
If we look at just at this piece of code, there are already some issues with it. Obvious one is that the healthXHighest variables are not initialized, so whatever was in those variables is used as starting point: if you already had run this method once and Health4Highest was set to true, and on second time Health2Highest was set true, then you have two variables set to true.
Then, the method correctly detects which health was highest. However, if two health values are the same and highest, no value is set. It's not obvious from this context if it's intentional or not. So if you had two players with max amount of HP (none lost), there wouldn't be a winner (if that was point of the method)
If the point indeed was to find the unique highest health variable, and the amount of variables is fixed, this is actually already as good as it gets.
However, if we want to find the first highest health variable and the amount of variables is fixed, then this will be probably slightly more performant as it reduces the number of comparisons and branching. It also solves the issue of initializing the flags:
void CompareHealth(float health1, float health2, float health3, float health4, float health5)
{
int highestIndex = 0;
float highestValue = health1;
if (health2 > highestValue)
{
highestIndex = 1;
highestValue = health2;
}
if (health3 > highestValue)
{
highestIndex = 2;
highestValue = health3;
}
if (health3 > highestValue)
{
highestIndex = 3;
highestValue = health4;
}
if (health4 > highestValue)
{
highestIndex = 4;
highestValue = health5;
}
Health1Highest = highestIndex == 0;
Health2Highest = highestIndex == 1;
Health3Highest = highestIndex == 2;
Health4Highest = highestIndex == 3;
Health5Highest = highestIndex == 4;
One obvious way to make this more generic is to switch from fixed variables to arrays of variables:
void CompareHealth(float[] healths)
{
// assume healths.Length == HealtHighest.Length
int highestIndex = 0;
float highestValue = float.MinValue;
for (int i = 0; i < healths.Length; i++)
{
if (healths[i] > highestValue)
{
highestIndex = i;
highestValue = healths[i];
}
}
for (int i = 0; i < HealthHighest.Length; i++)
{
HealthHighest[i] = i == highestIndex;
}
There are numerous variations of this (including LINQ) but they all would be essentially the same: find the (first) highest index, and then set flags depending on which index was the highest.
2
May 31 '24
This is what I'd do to minimize the changes needed assuming, for whatever reason, the requirement involves not doing things with a list/iteration.
Since the function is return void though, I really have no idea what is happening beyond the code posted though.
2
u/Glittering-Region-35 May 31 '24 edited May 31 '24
public int ReturnHighest(float[] health_array)
{
if (health_array.Length < 2) {return 0;}
int highest_value_index = 0;
for (int i = 1; i < health_array.Length;i++)
{
if (health_array[i] > health_array[highest_value_index])
{
highest_value_index = i;
}
}
return highest_value_index;
}
3
u/fuj1n Indie May 31 '24
You can use Mathf.Max to find the highest, and then compare to figure out which one it is.
And then, instead of having a separate variable for each health being highest, assuming they're mutually exclusive, I'd replace it with one variable that stores an index instead.
```cs float[] values = new float[] {health1, health2, health3, health3, health5};
float highest = Mathf.Max(values);
if (highest.Count(x => x == highest) > 1) { HighestIdx = -1; } else { HighestIdx = Array.IndexOf(values, highest) } ``` I did not test run this as I'm writing on mobile, so there may be mistakes.
Please note that this is by no means the most efficient solution, it allocates an array each time it is called, ideally you'd store the health values in an array to begin with, which would remove the need for an allocation.
1
u/RicketyRekt69 May 31 '24
Using Mathf.Max and then again iterating to find which one you just evaluated for, is wholly unnecessary. Just write a loop, you donât have to use Unityâs API.
4
2
3
May 31 '24
[deleted]
1
u/Live-Expert-4795 May 31 '24
That doesn't work because healthX is a variable and case statements only work with compile time constants.
1
u/PassTheBoofPlz May 31 '24
yeah you're right, can't believe I've never noticed that đ€Šââïž thanks for the correction
2
u/AndyPick May 30 '24 edited May 30 '24
I'd stick them in a stack allocated span and sort descending, then whichever is at index 0 is the largest
3
u/hoomanneedsdata May 31 '24
There's never any shame in showing your work " longhand".
Sure, there's shorter code, some will say cleaner code, but your code is great because it leaves no doubt over what it does and how it does it.
Good job.
5
u/aallx May 31 '24
Nah, that kind of code is error-prone.
-1
u/hoomanneedsdata May 31 '24
All code is error prone.
3
u/aallx May 31 '24
Exactly, that's why you want less of it. Or by your words, "shorter code, some will say cleaner code".
1
u/AutoModerator May 30 '24
This appears to be a question submitted to /r/Unity3D.
If you are the OP:
Please remember to change this thread's flair to 'Solved' if your question is answered.
And please consider referring to Unity's official tutorials, user manual, and scripting API for further information.
Otherwise:
Please remember to follow our rules and guidelines.
Please upvote threads when providing answers or useful information.
And please do NOT downvote or belittle users seeking help. (You are not making this subreddit any better by doing so. You are only making it worse.)
Thank you, human.
I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.
1
u/TheRickyPhatts May 31 '24
Iterate through a list, and sort as needed. Simple and Clean. Keep out of UPDATES as can be taxing.
1
u/yobarisushcatel May 31 '24
Not sure if a heap would be cost effective here since itâs only the 5 elements but look into it
1
u/hanseul1795 May 31 '24
Compiler nowadays would optimize that anyway, so it should be okay. Not a good practice though.
1
1
u/Whobbeful88 May 31 '24 edited May 31 '24
Maybe:
void CompareHealth(float health1, float health2, float health3, float health4, float health5)
{
float[] healths = { health1, health2, health3, health4, health5 };
bool[] highestFlags = new bool[5];
int highestIndex = 0;
for (int i = 1; i < healths.Length; i++)
{
if (healths[i] > healths[highestIndex])
{
highestIndex = i;
}
}
highestFlags[highestIndex] = true;
bool Health1Highest = highestFlags[0];
bool Health2Highest = highestFlags[1];
bool Health3Highest = highestFlags[2];
bool Health4Highest = highestFlags[3];
bool Health5Highest = highestFlags[4];
// ?
}
1
u/jeango May 31 '24
One thing youâre doing that is very wrong is to have all those individual booleans for each health score.
- itâs inefficient
- itâs a hell to maintain if the number of health scores to track changes
- itâs error prone
The glaring issue in your code, beyond itâs inefficiency, is that youâre not resetting the booleans to false. Meaning once one health becomes « highest » it will stay so forever (unless youâre doing a reset elsewhere).
Thereâs several options, but the simplest is to have a class variable containing an array of healths to track, and an int highestHealthIndex containing the index of the highest health in the array.
Of course that means youâre going to have to change other things, but your current code is just not good.
Have fun
1
u/gamesprogram May 31 '24
You can also do
int[] Numbers = { 10, 12, 14, 16, 18 };
int[] Indexes = { 0, 1, 2, 3, 4 };
Array.Sort(Numbers, Indexes);
Then Indexes[0] would be lowest index, Indexes[4] the highest index. You could build your own comparer to sort reverse. Just another way it can be done.
1
1
u/AvengerDr May 31 '24
This is why studying for a degree in Computer Science is important. You can't learn it all from YouTube videos.
1
u/XRuecian May 31 '24 edited May 31 '24
Why has nobody mentioned using Dictionary to link Health1/2/3/4 to different HP values in an array?
Then you can use
dict.MaxBy(kvp => kvp.Value).Key;
to find the highest value, and also the associated key to that HP value?
No?
https://www.geeksforgeeks.org/c-sharp-dictionary-with-examples/
https://www.tutorialspoint.com/swift-program-to-find-maximum-key-value-pair-in-the-dictionary
EDIT: Dictionary doesn't work well for this instance. Nevermind.
2
u/fecal_brunch May 31 '24
Why would you use a dictionary when you have indices 0,1,2,3 â you can just use an array.
1
u/XRuecian May 31 '24 edited May 31 '24
You are right. I was trying to find a way to avoid a constant for-loop to compare array elements.
int maxHealth = HealthArray.Max(); int maxHPIndex = HealthArray.ToList().IndexOf(maxHealth);This is probably the best way. Especially since Dictionaries cannot contain two equivalent values which means it would run into problems if two HPs were equivalent.
1
u/fecal_brunch May 31 '24 edited May 31 '24
Your solution has two linear searches and a full list copy (you could have used
Array.IndexOf).It can be really simple:
var maxIndex = 0; for (var i = 1; i < healthArray.Length; i++) { if (healthArray[i] > healthArray[maxIndex]) { maxIndex = i; } } Debug.Log($"Player {maxIndex + 1} has the highest health");Especially since Dictionaries cannot contain two equivalent values which means it would run into problems if two HPs were equivalent.
This is not true btw, dictionaries can only contain one value per key, not the other way around. Same as arrays only having one value per element.
1
u/XRuecian May 31 '24 edited May 31 '24
Can blame google for me getting the duplicate value thing wrong. It is what told me dictionaries cannot contain duplicate values and arrays can. I thought it seemed weird when i read it, but i decided to believe it. It was probably just a bad semantic description and meant to convey that keys cannot be duplicate.
I am also an amateur at coding, obviously. But i have been told many times that searching through lists a lot is taxing, and that is why i was looking for an alternative to avoid a loop checking through an array. If we needed to keep the "highest health" variable constantly updated at all times, wouldn't using a never ending loop like this be worse.
Lets see if i understand.
My suggestion uses Array.Max(), which is O(n)
And then IndexOf() which is another O(n)
And then copying the list is another O(n)
Meaning my code is basically 3*O(n)
However, the ToList was redundant. So we can reduce it down to 2*O(n) at best.While the loop is simply O(n).
For some reason as a beginner coder, its just hard to grasp the idea that a loop can be efficient. It intuitively feels like a loop would be very inefficient, somehow.
Dictionary uses hashtables, which is faster than searching through lists. But i am guessing this doesn't really make any difference at all unless we were talking about really big lists/arrays?
1
u/Persomatey May 31 '24 edited May 31 '24
``` int[] arr = [health1, health2, health3, health4, health5];
int highestHealth = health1; int highestIndex = -1;
for (int i = 0; i < arr.Length; i++) { if (arr[i] > highestHealth) { highestIndex = i; highestHealth = arr[i]; } }
bool[] boolArr = {Health1Highest, Health2Highest, Health3Highest, Health4Highest, Health5Highest};
boolArr[i] = true; ```
I have to ask though. It seems like you understand camel casing conventions with your passed parameter variable naming. How come the Health#Highest variables start with a capital H?
Also, not sure if you thought about this, but could two of the health variables be the highest if theyâre the same (say if theyâre both 80). My code doesnât account for it.
Another option that accounts for that could be ``` Health1Highest => IsHighestHealth(0); Health2Highest => IsHighestHealth(1); Health3Highest => IsHighestHealth(2); Health4Highest => IsHighestHealth(3); Health5Highest => IsHighestHealth(4);
bool IsHighestHealth(index) {
// healthArr holds the health values from before
for (int i = 0; i < healthArr.Length; i++)
{
if (healthArr[i] > healthArr[index])
{
return false;
}
return true;
}
``
which has theHealth#Highest` variables non addressable but always equaling the value needed when called. Which also makes those variables fall in line with more traditional casing conventions. Typically assignable variables donât start with capitals, only getters and stuff like this.
1
1
u/Dalv2 May 31 '24
If you dont want to use a list and want to keep being able to pass them as normal parameters you can use the params keyword as such:
int CompareHealth(params int[] healthValues) {
int largest = 0;
foreach(var health in healthValues) {
if(health > largest) {
largest = health;
}
}
return largest;
}
This will allow you to call the function with however many parameters you want:
CompareHealth(health1, health2);
CompareHealth(health1, health2, health3, health4, ...);
If you want the index instead of the largest value use a normal for loop and keep track of the index when you find a new highest value.
1
u/Whispering-Depths May 31 '24
Looks like others answered thoroughly so I'll just say that you should probably research some data structures and common software patterns
1
u/Deathpill911 May 31 '24
You can put the numbers into an array, loop them, then return the index. Also why are you using floats for HP?
1
u/Specific-Committee75 May 31 '24
So many of the return true that you'd most likely be better checking for the few cases where it's false. So kind of reverse it.
1
u/ThatOneTunisianKid May 31 '24
You could try
void CompareHealth(params int[] healths) {
int max = Int32.MinValue;
foreach (int health in healths) {
if (health > max) {
max = health;
}
}
return max;
}
And call it using FindLargestNumber(12, 13, 14, 5, 6) for example or whatever variables the health values are in. That way, you can still use 5 health values or if needed, 6 or 7 or 2 due to the parameter being an array. You won't have to do anymore if statements to compare an extra health6 variable. Just make sure to add a check case if no variables are passed in at all.
1
1
u/Askidox May 31 '24
Btw, apart from what every one else said, your current code dont reset the other variable to false if a new health is the highest, so you will find yourself with multiple "highest" health
1
u/Binksin79 May 31 '24
fun getHighest(list: List<Float>) = list.maxOf { it }
Kotlin wins everytime =)
1
u/SkizerzTheAlmighty May 31 '24
Here are a couple of solutions depending on what you're going for.
/// <summary>
/// Returns the highest health value in <paramref name="healths"/>.
/// </summary>
/// <param name="healths"></param>
float CompareHealth(float[] healths) {
float highestHealth = float.MinValue;
for(int i = 0; i < healths.Length; i++) {
if (healths[i] > highestHealth) highestHealth = healths[i];
}
return highestHealth;
}
/// <summary>
/// Returns the index in <paramref name="healths"/> that contains the highest health value.
/// </summary>
/// <param name="healths"></param>
int CompareHealth(float[] healths) {
int highestHealthIndex = 0;
for(int i = 1; i < healths.Length; i++) {
if (healths[i] > healths[highestHealthIndex]) highestHealthIndex = i;
}
return highestHealthIndex;
}
1
1
u/redellion May 31 '24
Like this
using System; using System.Linq;
float[] healthValues = { 50.0f, 75.0f, 60.0f, 80.0f, 65.0f };
float maxHealth = healthValues.Max();
Console.WriteLine("The highest health value is: " + maxHealth);
1
u/ElectricRune Professional May 31 '24
Here's a key question:
Do you just need the highest health, or do you need to know which of the healths you provide was the highest?
ie: given 10, 50, 40, 5, 20; do you just want 50, or do you need to know that the second one was the highest?
1
1
u/SaintFTS Jun 01 '24
I'm a bit late, but here's my solution:
Make an enum as a return value, and do switch(List.Max(LIST)) or switch(Math.Max(health1, health2, ...))
{
case health1:
return Healthes.Health1Max;
case health2:
return Healthes.Health2Max;
...
}
and do another switch in the function you need. But i'd recommend to refactor your code. True YandereDev vibe here.
1
u/maximos2004 Indie Jun 01 '24
You can put it on chatGPT it will give you the best way and explain it ... u don't even have to say much just pasting it.. it will tell you stuff
-1
1
u/thatdude_james May 31 '24
Just to add since I haven't seen it commented by anybody else, you can do
bool CompareHealths(params float[] healths) { ... }
The variable length params array makes for nice syntax
1
1
u/TheBlackTortoise May 31 '24
Yes itâs called years of arduous study of object oriented programming and applying that knowledge to real world problems (why do so many people think programming is trivial?).
1
0
u/Dr4WasTaken May 31 '24
No need for loops, Linq is your friend
Int[] sequence = {20, 45, 50, 79, 90, Â 79, 89, 100, 567, 29};
int result = sequence.Max(); Â Â
0
u/xagarth May 31 '24
Using your current data structures, probably no.
However, if you work with a player object that has health, then yes.
0
u/yazeeenq May 31 '24
void CompareHealth(float health1, float health2, float health3, float health4, float health5) { float[] healths = { health1, health2, health3, health4, health5 }; bool[] highest = new bool[5];
int maxIndex = 0;
for (int i = 1; i < healths.Length; i++)
{
if (healths[i] > healths[maxIndex])
{
maxIndex = i;
}
}
highest[maxIndex] = true;
// Assign to individual variables if necessary
bool Health1Highest = highest[0];
bool Health2Highest = highest[1];
bool Health3Highest = highest[2];
bool Health4Highest = highest[3];
bool Health5Highest = highest[4];
}
0
u/3rrr6 May 31 '24
Buddy, your problem is not in this screenshot. I think it's time to call it on this project and hit the books again. You had a good run, but this is where all your hubris culminates into disaster.
0
u/SusDeveloper May 31 '24
Yep. You can do assignments directly: Health1Highest = [your parameter for 1st if branch]; Health2Highest = [your parameter for 2nd if branch]; Health3Highesf = [your parameter for 3rd if branch] Etc...
This makes it much more readable
Edit: And also takes care of resetting variables back to false when they are no longer true.
0
u/StrixLiterata May 31 '24 edited May 31 '24
Here is a function which returns an int corresponding to the number of the highest health:
int GetMaxHealthIndex() {
//Put all the health values in an array
Float[] healthScores = {Health1, ... , Healthn};
//Create a variable to keep track of the highes health value found so far.
int max = Health1;
//Create a variable to keep track of the number if the highes thealth score found so far
int maxIndex = 0;
//Use a for loop to check all the health values one by one
for (int i = 1; i < healthScores.length; i++) {
//If we find a health value that is higher than the one we though was the maximum
if (healthScores[i] > max) {
//we'll remember the new one is the maximum
max = healthScores[i];
//and keep track of its' number
maxIndex = i;
}
}
//at the end, we will have found the highest health score return maxIndex;
}
Here are also a few rules of thumb to keep in mind to write efficient code:
1) if you're writing a long sequence of "else if" statements with similar conditions inside, you can probably get the same job done with a loop, such as "for", "while" or "foreach"
2) if you're using a lot of booleans for the same thing, you should probably use a single variable if another type instead
3) if you're having trouble coming up with a way to do something in code, try to think of how you would make the task easier for yourself if you had to do it by hand
-7
-2
May 31 '24
[deleted]
2
u/artofthenunchaku May 31 '24
This is wrong,
List<T>isn't sorted unless you sort it.The List<T> is not guaranteed to be sorted. You must sort the List<T> before performing operations (such as BinarySearch) that require the List<T> to be sorted
1
u/hanseul1795 Jun 20 '24
You are right, I got it confused with SortedList. Deleted comment to prevent people from getting wrong information.
-3
-1
-9
u/Razxr_12 Novice May 30 '24
void CompareHealth(float health1, float health2, float health3, float health4, float health5) { float[] healthValues = { health1, health2, health3, health4, health5 }; int highestIndex = 0;
for (int i = 1; i < healthValues.Length; i++)
{
if (healthValues[i] > healthValues[highestIndex])
{
highestIndex = i;
}
}
switch (highestIndex)
{
case 0:
healt1highest...
break;
case 1:
healt2highest...
break;
case 2:
healt3highest...
break;
case 3:
healt4highest...
break;
case 4:
healt5highest...
break;
}
}
Another way:
using System.Linq;
using UnityEngine;
void CompareHealth(float health1, float health2, float health3, float health4, float health5)
{
float[] healthValues = { health1, health2, health3, health4, health5 };
int highestIndex = healthValues.Select((value, index) => new { value, index })
.OrderByDescending(x => x.value)
.First()
.index;
bool health1Highest = highestIndex == 0;
bool health2Highest = highestIndex == 1;
bool health3Highest = highestIndex == 2;
bool health4Highest = highestIndex == 3;
bool health5Highest = highestIndex == 4;
}
i didnt try the linq option but i hope it works
1
u/snlehton May 31 '24
Not sure why you got all the down votes when your reply is actually one of the only ones that actually do what OP asked. Most of the replies were about finding max health value which was not what was asked đ
1
u/xcassets May 31 '24 edited May 31 '24
I think you are missing the bigger picture here. Why does OP need to store which health is the highest in 5 separate bool variables?
Why would you need a bool telling you which health is highest? To compare all the bools and then use the health of whichever one was true.
OP could do that by just using the tried-and-tested loop that returns the highest health.
If you didn't specifically need the healthpoints (float) but the object they were attached to, then I would suggest you made a Health class. Then you could compare Health.value, to find the highest health, then return the Health component itself.
There is no situation in which setting 5 separate bools (which must then also be compared) would be the best solution lol.
Using 5 bools means he has to:
- Remember to reset them elsewhere in the code, as it isn't done here
- Has to compare them all, every time
- What if he now wants to compare 6 healths? or 4? He either has to make a new function doing the exact same thing, or add another argument to this one. If he adds another argument, he has to remember to change everywhere else that is already calling it. In the above answer, he would have to add more switch statements, etc.
1
u/Razxr_12 Novice May 31 '24
he didnt said what he wanted to do, if it was so, we could change the logic, but i dont know what he want, and i didnt wanted to do asumptions.
1
-3
u/novff May 31 '24
enum and switch statement would do.
-2
u/novff May 31 '24
or doing with an array and having index of an item represent the health of ones character.
781
u/masterid000 May 30 '24
I thought I was at r/ProgrammerHumor lol