Tbh, excepted the weird indentation on the second half from the autoformatting trying to do its best, it looks worse than it reads.
I wouldn't write that, but that kind of imbricated ternary conditional operator, at least each imbricated if is just cascading with the same check for a different value into the next one, wouldn't do it, but seen way worse, way way worse with those operators.
Indent it nicer with one condition & value per line and it's fine
Or even better: Define the values somewhere where they belong - probably as an array - and move this logic to a dedicated function/method. Also obviously use a for loop instead of endless ifs.
Much cleaner. And won't get you moved to QA.
This is game code, your expectations are way too high and bold to assume there is QA xD
Or just list comprehension
``` values .takeWhile(range -> range < mk) .count()
Format it like this and it's just shorter else-if. Aside from not doing binary search (if you want proper formatting: with an array), I'd give it a pass.
float perXpBoost =
mk>10000? 1f:
mk>7500? 0.9f:
mk>5000? 0.8f:
mk>2500? 0.6f:
....;
Binary search probably would be slower here actually, given how small the input size would be
If you just stapled the algorithm yes, but I'd organize comparisons like below to still get fewer of them than an else-if chain. Of course we need to look at the generated assembly to verify that this is actually faster after optimizations. (Edit: Btw stuff like that is why basic algorithms are always nice to know.)
if(mk>middle_option) {
if(mk>three_fourths_option)...
else ...
}
else {
if(mk>one_fourth_option)...
else ...
}
The lack of spaces is pretty weird there
The urge to use tertiary, whether it's necessary or not.
I must admit I am a fan of tertiary but never go under a level of nested tertiary.
Ternary*
I genuinely thing there’s nothing wrong with this code, besides the weird indentation. Something like this would be completely readable to anyone who has programmed for more than 2 weeks:
float petXpBoost =
mk > 10000 ? 1.0f
: mk > 7500 ? 0.9f
: mk > 5000 ? 0.8f
/* … */
: 0.1f;
here, have something far worse: a 32-line, seven-level nested ternary with complex conditions
this is a simple truth tables two way comparison and a three way comparison-
IsCooldown(a, b):
a | b | result |
---|---|---|
F | F | a.ActionID == original ? a : b |
T | F | b |
F | T | a |
T | T | hasCharges(a, b) |
hasCharges(a,b):
a | b | result |
---|---|---|
F | F | max(a.cooldownRemaining, b.cooldownRemaining) |
T | F | chargeRace(a, b) |
F | T | chargeRace(b, a) |
T | T | chargeComparison(a, b) |
chargeRace(offCooldown, onCooldown):
offCooldown.remainingCharges > 0 ? offCooldown : min(offCooldown.ChargeCooldownRemaining, onCooldown.cooldownRemaining)
chargeComparison(a, b):
symbol | result |
---|---|
== | min(a.ChargeCooldownRemaining, b.ChargeCooldownRemaining) |
> | a |
< | b |
/// <summary>
/// Compares two action cooldown states and returns the one considered "better" based on cooldown timers and remaining charges.
/// </summary>
/// <param name="original">The original action ID to prioritize when neither cooldown is active.</param>
/// <param name="a">The first action cooldown tuple containing an action ID and its cooldown data.</param>
/// <param name="b">The second action cooldown tuple containing an action ID and its cooldown data.</param>
/// <returns>
/// Returns the tuple (ActionID, CooldownData) representing the preferred cooldown state.
static (uint ActionID, CooldownData Data) Compare(
uint original,
(uint ActionID, CooldownData Data) a,
(uint ActionID, CooldownData Data) b
) {
Func< (uint ActionID, CooldownData Data),
(uint ActionID, CooldownData Data),
(uint ActionID, CooldownData Data)>
chargeCompare = (x, y) =>
x.Data.RemainingCharges.CompareTo(y.Data.RemainingCharges) switch {
0 => new[] { x, y }.MinBy(z => z.Data.ChargeCooldownRemaining),
1 => x,
-1 => y,
_ => throw new InvalidOperationException("Unexpected comparison result")
};
Func< (uint ActionID, CooldownData Data),
(uint ActionID, CooldownData Data),
(uint ActionID, CooldownData Data)>
chargeRace = (offCooldown, onCooldown) =>
offCooldown.Data.RemainingCharges > 0
? offCooldown
: offCooldown.Data.ChargeCooldownRemaining < onCooldown.Data.CooldownRemaining
? offCooldown : onCooldown;
var hasCharges = new Dictionary<(bool, bool), Func<(uint, CooldownData)>>() {
[(false, false)] = () => new[] { a, b }.MaxBy(x => x.Data.CooldownRemaining),
[(true, false)] = () => chargeRace(a, b),
[(false, true)] = () => chargeRace(b, a),
[(true, true)] = () => chargeCompare(a, b)
};
var isCooldown = new Dictionary<(bool, bool), Func<(uint, CooldownData)>>() {
[(false, false)] = () => a.ActionID == original ? a : b,
[(true, false)] = () => b,
[(false, true)] = () => a,
[(true, true)] = () => hasCharges[(a.Data.HasCharges, b.Data.HasCharges)]()
};
return isCooldown[(a.Data.IsCooldown, b.Data.IsCooldown)]();
}
You people are being dramatic lol this is fine and readable and totally fast enough for a list that short
So many magic numbers. What's 0.1f? Why are we counting from 1 to 6 but only have 5 checks? It's so unintuitive
Anyway
``` mkRanges = [25, 100, 250, 500, 1000]
rangesPassed = len(takewhile(lambda range: range < mk, mkRanges))
petXpBoost = 0.1 * (rangesPassed + 1)
Edit- just noticed that's not python. I'll assume java?
int[] mkRanges = {25, 100, 250, 500, 1000};
int rangesPassed = Arrays.stream(mkRanges) .takeWhile(range -> range < mk) .count();
float petXpBoost = 0.1f * (rangesPassed + 1);
Or c++
std::vector<int> mkRanges = {25, 100, 250, 500, 1000};
auto it = std::find_if_not(mkRanges.begin(), mkRanges.end(), [&](int range) { return range < mk; }); int rangesPassed = std::distance(mkRanges.begin(), it);
float petXpBoost = 0.1f * (rangesPassed + 1); ```
The structure doesn't change
Being explicit is more readable, and much easier to modify. Shorter code is not always better.
Avoiding any computations makes the code faster. If this is called in some hot loop it could make a difference.
So the shown code is actually better than parents solution. Just that it's formatted in a weird way.
avoiding computation? you have exactly the same amount of branches. takewhile is short circuiting
what is not explicit about reading the threshold of ranges? its much clearer why a specific value is that value, instead of having 5 different magic numbers
This is so much worse than the original code whose intent is clear and easier to instantly reason about
It hurts so much to look at and the fact that it's just
If mk > 10000 → boost = 1.0 Else if mk > 7500 → boost = 0.9 Else if mk > 5000 → boost = 0.8 Else if mk > 2500 → boost = 0.7 Else if mk > 1000 → boost = 0.6 Else if mk > 500 → boost = 0.5 Else if mk > 250 → boost = 0.4 Else if mk > 100 → boost = 0.3 Else if mk > 25 → boost = 0.2 Else → boost = 0.1
Who wrote those, and how bad were you abused growing up to wish this pain for other peopleJust use an ordered map, ffs. All these programmers whose only data structures are a hash map and an array have so much to learn.
Imo would be probably better like this?
If above 1k Boost = Mk%2500/10+0.6
Else if above 100 Boost = Mk%250/10+0.3
Else if > 25 boost 0.2 Else boost =0.1
Tho I think there should be a way to get that under a specific formula.
Edit: Deep seek recommendation is lookup table. Damn that thing looks MUCH better. And easier to deal compared to that shit.
its literally a threshold accumulation. for each value above an arbitrary number, the "boost" jump in values of 0.1
just count how many thresholds you reach, and multiply by 0.1
Yeah also possible
Jesus fuck, just write a switch at this point.
Good heavens no. A table with entries of the limit and the body with a simple algorithm that walks through it.
Some people just want to see the world burn.
How is this painful?? Very simple code, easy to read and totally find for such a short list... id consider anything else to be over-engineering