3
\$\begingroup\$

I have a private method in my class to update user experience.

First of all, I create giveExp based on contentLength (string.length)

In that method I had formula for count level from exp: (Math.sqrt(2 * user.xp - 175) + 25) / 10;

I also need to inform the player that they have leveled up.

Full code

private updateXp(
        contentLength: number,
        channel: Readonly<WritableChannel>,
        xp: string
    ) {
        const giveExp = Math.round(contentLength * 3 * 0.1);
        let userXp = Number.parseInt(xp, 10);
        const level = (Math.sqrt(2 * userXp - 175) + 25) / 10;
        userXp += giveExp < 15 ? giveExp : 15;
        const nowLevel = (Math.sqrt(2 * userXp - 175) + 25) / 10;
        if (level.toFixed(0) < nowLevel.toFixed(0)) {
            void channel.send("Level up");
        }
        return String(userXp);
    }
\$\endgroup\$
7
  • \$\begingroup\$ You have a bug in line if (level.toFixed(0) < nowLevel.toFixed(0)) {. toFixed converts a number to a string and string compare uses character codes from left to right to check if GT or LT (< >) thus "9" < "10" is false which I don't think is your intent. \$\endgroup\$
    – Blindman67
    Mar 14, 2022 at 12:57
  • 1
    \$\begingroup\$ Why are xp and the return value strings? \$\endgroup\$
    – xehpuk
    Mar 14, 2022 at 20:00
  • \$\begingroup\$ @xehpuk Because when a number becomes very large, it can work only as BigInt. I also have a string type in my database. \$\endgroup\$ Mar 15, 2022 at 8:10
  • \$\begingroup\$ @KristalkillPlay Then your code and the accepted version are broken (Number.parseInt). And you should consider using a numeric type for your persistence. \$\endgroup\$
    – xehpuk
    Mar 15, 2022 at 13:24
  • \$\begingroup\$ @xehpuk: Why are they broken? The parseInt function converts its first argument to a string, parses that string, then returns an integer or NaN. parseInt converts a BigInt to a Number. The JavaScript Number type is a double-precision 64-bit binary format IEEE 754 value. MDN: JavaScript \$\endgroup\$
    – peterSO
    Mar 15, 2022 at 15:01

1 Answer 1

3
\$\begingroup\$

I found your code hard to read.

In comments, the OP said: "when a number [xp] becomes very large, it can work only as BigInt. I also have a string type in my database."

The parseInt function converts its first argument to a string, parses that string, then returns an integer or, if greater than Number.MAX_VALUE, Infinity. A Number is converted to fixed-point notation, except large numbers (more than 20 digits) are converted to exponential notation. A BigInt is converted to fixed-point notation. The parseInt function returns only the integer portion of exponential notation. Parse xp as BigInt(xp).

Factor out the level formula to make it obvious that you are using the same formula. For a comparison of integer numeric values, don't compare strings. Use Math.round.

JavaScript Number does not support complex numbers. The level formula expression Math.sqrt(2 * xp - 175) propagates NaN if (2 * xp - 175) < 0 or xp < 87.5. Math works with the Number type. The level formula expression Math.sqrt(2 * xp - 175) only handles xp values less than or equal to (Number.MAX_VALUE / 2) or 8.988465674311579e+307.

Binary floating-point numbers (IEEE 754) are an approximation, including the value 0.1. For a more exact result, don't multiply by 0.1, divide by 10.

Reorder code to group related statements together.

Sometimes you used xp, sometimes you used exp. Use xp (or exp) consistently.

You have level and nowLevel. For clarity, use oldLevel and newLevel.

For a math formula, use Math.min instead of the conditional (ternary) operator.

Since the action is "Level up", reverse the test for up to newLevel > oldLevel.

For minified code, use spacing and indentation to enhance readability.

function updateXp(
        contentLength,
        channel,
        xp) {

    function levelFormula(xp) {
        return Math.round((Math.sqrt(2 * xp - 175) + 25) / 10);
    }

    let userXp = Number.parseInt(BigInt(xp), 10);
    const oldLevel = levelFormula(userXp);

    const giveXp = Math.round(contentLength * 3 / 10);
    userXp += Math.min(giveXp, 15);
    const newLevel = levelFormula(userXp);

    if (newLevel > oldLevel) {
        void channel.send("Level up");
    }

    return String(userXp);
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I will point out that dividing is a much slower operation that multiplying (an order of magnitude slower in some cases). Also in this case there is no need to scale Xp as OP is only comparing levels. \$\endgroup\$
    – Blindman67
    Mar 14, 2022 at 17:37
  • \$\begingroup\$ @Blindman67: I am well aware of machine instruction times. Intel rates its top Core processors at over 200 GFLOPS. Correct numerical analysis is more important than a few nanoseconds. "premature optimization is the root of all evil." - Sir Tony Hoare. \$\endgroup\$
    – peterSO
    Mar 15, 2022 at 4:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.