9
\$\begingroup\$

I've been working on a personal project. There's this game called TFT which is an autochess game. Each match is played by 8 players and there's a shared pool of champions. Each champion has their cost and how many of each there are, below is the table:

12 Champs that cost 1 gold, 39 of each champ.

12 Champs that cost 2 gold, 26 of each champ.

12 Champs that cost 3 gold, 18 of each champ.

9 Champs that cost 4 gold, 13 of each champ.

6 Champs that cost 5 gold, 10 of each champ.

So, I've been doing some functions to get the probability of getting x champ in a 5 champion deck, something I forgot to say is that in the game, you have your player level which somewhat influences in the pool, below is the multiplicator array, each row is a player level starting from Level 2 (which is the zeroth row) to Level 9, and each column is the champions cost (cost 1 to 5).

// pool.class.js
class Pool {
    constructor(champPool) {
        this.champPool = champPool; //this is an array of Champions(name,pool,cost)
        this.chancePerTier = [
            [1, 0, 0, 0, 0], //Level 2
            [0.7, 0.25, 0.05, 0, 0], //Level 3
            [0.5, 0.35, 0.15, 0, 0], //Level 4
            [0.35, 0.35, 0.25, 0.05, 0], //Level 5
            [0.25, 0.35, 0.3, 0.1, 0], //Level 6
            [0.2, 0.3, 0.33, 0.15, 0.02], //Level 7
            [0.15, 0.2, 0.35, 0.24, 0.06], //Level 8
            [0.1, 0.15, 0.3, 0.3, 0.15] //Level 9
        ];
    }

    getChampProbability(champName, playerLevel) {
        let poolSize = this.getPoolBasedOnLevel(playerLevel);
        let poolIndex = this.champPool.findIndex(function(champ) {
            return champ.getName() === champName;
        });
        if (poolIndex === -1) return 0;
        let champPool = this.champPool[poolIndex].getPool();
        let champCost = this.champPool[poolIndex].getCost();

        return (
            (champPool / poolSize) *
            5 *
            100 *
            this.chancePerTier[playerLevel - 2][champCost - 1]
        ).toFixed(2);
    }

    getPoolBasedOnLevel(summonerLevel) {
        let pool;
        if (summonerLevel == 2) {
            pool = this.champPool.reduce(function(prevChamp, currentChamp) {
                if (currentChamp.getCost() == 1) {
                    return prevChamp + currentChamp.getPool();
                } else {
                    return prevChamp;
                }
            }, 0);
        } else if (summonerLevel == 3 || summonerLevel == 4) {
            pool = this.champPool.reduce(function(prevChamp, currentChamp) {
                if (currentChamp.getCost() >= 1 && currentChamp.getCost() <= 3) {
                    return prevChamp + currentChamp.getPool();
                } else {
                    return prevChamp;
                }
            }, 0);
        } else if (summonerLevel == 5 || summonerLevel == 6) {
            pool = this.champPool.reduce(function(prevChamp, currentChamp) {
                if (currentChamp.getCost() >= 1 && currentChamp.getCost() <= 4) {
                    return prevChamp + currentChamp.getPool();
                } else {
                    return prevChamp;
                }
            }, 0);
        } else {
            pool = this.getWholePool();
        }
        return pool;
    }

    getWholePool() {
        let allPool = this.champPool.reduce(function(prevChamp, currentChamp) {
            return prevChamp + currentChamp.getPool();
        }, 0);
        return allPool;
    }
}
module.exports = Pool;
// champion.class.js
class Champion {
    constructor(name, pool, cost, origin) {
        this.name = name; // String
        this.pool = pool; // Integer
        this.cost = cost; // Integer
        this.origin = origin; // Origin
    }

    getName() {
        return this.name;
    }

    setPool(newPool) {
        this.pool = newPool;
    }

    getPool() {
        return this.pool;
    }

    getCost() {
        return this.cost;
    }

    getOrigin() {
        return this.origin;
    }
}

module.exports = Champion;

The code is working, but I think there's a good-looking approach to this ugly mess.

\$\endgroup\$
  • 1
    \$\begingroup\$ Honestly, the only part of it all that I think could be improved (beyond comments in your code) is to use a switch/case instead of the if/else waterfall. Switch/case logic is a lot easier to read when you've got straightforward logic as you have here. \$\endgroup\$ – technicallynick Nov 27 '19 at 3:51
1
\$\begingroup\$

As was already mentioned in the comments, a switch statement could be used to clean up the sets of if statements. Another option would be to abstract the similar callback functions passed to .reduce()- perhaps using partially applied functions to accept parameters - e.g. the minimum values in conditions like currentChamp.getCost() <= 3.

It seems like let is used for most variables. It is wise to default to using const to avoid accidental re-assignment. When you decide you do need to reassign a value then use let.

The method Pool::getChampProbability() could use this.champPool.find() instead of this.champPool.findIndex(), since the index is only used to dereference an array element. This would mean 0 should be returned if find() returned undefined (instead of if (poolIndex === -1) return 0;).

The method Pool::getWholePool() has a single use variable- i.e. allPool. That variable can be eliminated. Use of a linter would help find things like this.

The else keywords can be eliminated in many spots following a return in a conditional e.g. many of the else’s in Pool::getPoolBasedOnLevel()

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

getPoolBasedOnLevel() is the oddity here. There are three arrow functions doing similar jobs. Using one function with a lookup should clean things up. Effectively, each of them is doing this:

const sum = list=>list.reduce((a,b)=>a+b,0)

function poolTotal(champ_pool,min_cost,max_cost) {
    const champs_for_level = champ_pool.filter(champ=>
        champ.getCost()>=min_cost &&
        champ.getCost()<=max_cost
    );
    const pools = champs_for_level.map(
        champ=>champ.getPool()
    )
    return sum(pools);
}

That function also implicitly determines the min and maxchampion cost for each level. It would benefit readability to state this is explcitly.

function costsForLevel(level) {
    if (level===2) return {min:1,max:1}
    if (level===3 || level===4) return {min:1,max:3}
    if (level===5 || level===6) return {min:1,max:4}
    return {min:0,max:Number.MAX_VALUE}
}

With both of these functions defined, then getPoolBasedOnLevel() can be written as so

getPoolBasedOnLevel(summonerLevel) {
    const {min,max} = costForLevel(summonerLevel);
    return poolTotal(this.champPool,min,max);
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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