grouping people with the same score

Question

I made the code underneath to sort my array and group people with the same score to show who all got 1st, 2nd and 3rd.

the "points" variable looks like this

points: {
  '204255221017214977': 3,
  '208993609255485441': 5,
  '382197942762602496': 4,
  '664606841409634324': 5,
  '779857709155090432': 4,
  '381084508276916225': 5,
  '794200859612545075': 4,
  '155149108183695360': 4,
  '438978127973318656': 3,
  '304028235082104843': 2,
  '174548757151481856': 1
}

I was wondering if there is a much easier way to sort and put together the string like I did underneath with much easier code. Cuz I'll be honest this code looks and feels aweful.

example endresult:

:first_place: @Excanator had 5 points
:first_place: @zerow had 5 points
:first_place: @AnimeHotOwO had 5 points

:second_place: @christmas beans had 4 points
:second_place: @ExcavatorGames had 4 points
:second_place: @Dyno had 4 points
:second_place: @LogiBot Premium had 4 points

:third_place: @Restarter v3 had 3 points
:third_place: @YAGPDB.xyz had 3 points

@aNt had 2 points
@Kaori had 1 point

const sorted = Object.keys(points).sort((a, b) => {
    return points[b] - points[a]
})

var results = `\n`

var firstPlace = points[sorted[0]]
var secondPlace = null
var thirdPlace = null
var leftover = null

for(const key of sorted) {
    var amount = points[key]
    if(leftover){
        results += `<@${key}> had ${amount} point${amount === 1 ? '' : 's'}\n`
    } else if(thirdPlace) {
        if(thirdPlace === amount) {
            results += `:third_place: <@${key}> had ${amount} point${amount === 1 ? '' : 's'}\n`
        } else {
            results += `\n`
            results += `<@${key}> had ${amount} point${amount === 1 ? '' : 's'}\n`
            leftover = amount
        }
    } else if(secondPlace) {
        if(secondPlace === amount) {
            results += `:second_place: <@${key}> had ${amount} point${amount === 1 ? '' : 's'}\n`
        } else {
            results += `\n`
            results += `:third_place: <@${key}> had ${amount} point${amount === 1 ? '' : 's'}\n`
            thirdPlace = amount
        }
    } else if(firstPlace) {
        if(firstPlace === amount) {
            results += `:first_place: <@${key}> had ${amount} point${amount === 1 ? '' : 's'}\n`
        } else {
            results += `\n`
            results += `:second_place: <@${key}> had ${amount} point${amount === 1 ? '' : 's'}\n`
            secondPlace = amount
        }
    }
}
```

Answer 1

Functions

Always write code as functions. Even when its just an example, writing code as a function gives it a name, and make you think in terms of that name.

Functions should be as simple as possible doing one thing only. (see rewrite)

Review

• USE SEMICOLONS!!!
• Don't add code that is redundant or superfluous. Eg var secondPlace = null can be var secondPlace.
• Keep it DRY. (Don't Repeat Yourself). You have a lot of repeated code that does similar or the same. Using function will reduce a lot of repeated code.
• Use constants to define magic values. (see rewrite)
• Use an Array to hold lists of strings then use Array.join to join them using the delimiter (see rewrite)
• Use appropriate names. Names define the abstractions you use, poor naming make code harder to understand especially when a single abstract concept gets many names, or the same name represent multiple abstractions.
  • points As records should be results
  • key could be id.
  • amount could be score

Rewrite

Rewrite is just an example as there are many ways to do this.

Functions

The rewrite breaks the task into 3 parts.

• groupByScore groups by score. Using a Map indexed by score to hold arrays of ids with each score. Returns the groups as the Map
• sortByScore Takes the grouped results and sorts by score returning an array
• resultsByPlace Takes an array of grouped results and returning an array of strings each containing the place, id, and score.

Constants

There are two constants that remove magic values from the code and moves them to one place at the top of the code.

One of the constants is defined as a function score2Str and created the string eg " has 1 point"

Util functions

Common tasks can be written as utility functions. These function can be part of a library of functions you can use within other projects. There is one Util function pluralize that adds a "s" when a value is not != 1. It is defined as a tagged template and can be used as a tagged template

pluralize` point${score}`

Or called as a function

pluralize([` point`], score)

As modules

Thus we get the code as

File strings/utils.jsm

const pluralize = (strs, value) => strs[0] + (value != 1 ? "s" : "") + (strs[1] ? strs[1]: "");
export {pluralize};

File studentResults.jsm

import {pluralize} from "./strings/utils.jsm";

const PLACINGS = ":First Place: ,:Second Place: ,:Thrid Place: ".split(",");
const score2Str = score => " has " + score +  pluralize` point${score}`;

function groupByScore(students) {
    const scoreGroups = new Map();
    Object.entries(students).forEach(([id, score]) => {
        const group = scoreGroups.get(score);
        if (group) { group.push(id) }
        else { scoreGroups.set(score, [id]) }
    });
    return scoreGroups;
}    
function sortByScore(scoreGroups) {
    return [...scoreGroups.entries()].sort((gA, gB) => gB[0] - gA[0]);
}    
function resultsByPlace(sorted, cats = PLACINGS, scoreStr = score2Str) {
    var idx = 0;
    const results = [];
    for (const [score, group] of sorted) {
        const cat = idx < cats.length ? cats[idx ++] : (idx ++, "");
        results.push(...group.map(id => cat + id + scoreStr(score)));
    }
    return results;
}
export {groupByScore, sortByScore, resultsByPlace};

As running example.

As CodeReview snippets do not support modules below is a working example compatible with CodeReview snippets.

Code

const pluralize = (strs, value) => strs[0] + (value != 1 ? "s" : "") + (strs[1] ? strs[1]: "");
const tag = (tag, props = {}) => Object.assign(document.createElement(tag), props);
const append = (par, ...sibs) => sibs.reduce((p, sib) => (p.appendChild(sib), p), par);

// Rewrite
const studentScores = {'Adam': 3, 'Bill': 5,'Cindy': 4, 'Dafney': 5, 'Eric': 4, 'Fred': 5, 'Graham': 4, 'Hillary': 4, 'Ian': 3, 'Jill': 1, 'Kev': 0};
const PLACINGS = ":First Place: ,:Second Place: ,:Thrid Place: ".split(",");
const score2Str = score => " has " + score +  pluralize` point${score}`;

const results = resultsByPlace(sortByScore(groupByScore(studentScores)));
append(document.body, ...results.map(result => tag("div",{textContent: result})));
function groupByScore(students) {
    const scoreGroups = new Map();
    Object.entries(students).forEach(([id, score]) => {
        const group = scoreGroups.get(score);
        if (group) { group.push(id) }
        else { scoreGroups.set(score, [id]) }
    });
    return scoreGroups;
}
function sortByScore(scoreGroups) {
    return [...scoreGroups.entries()].sort((gA, gB) => gB[0] - gA[0]);
}
function resultsByPlace(sorted, cats = PLACINGS) {
    var idx = 0;
    const results = [];
    for (const [score, group] of sorted) {
        const cat = idx < cats.length ? cats[idx ++] : (idx ++, "");
        results.push(...group.map(id => cat + id + score2Str(score)));
    }
    return results;
}

Answer 2

Your code would benefit from separating data and display. Results can also be constructed by using the .join() method on an array, reducing the need for checking boundaries. I'm not sure that these modifications resulted in "easier" code, but the singular point of truth makes it harder for bugs to hide.

const points = {
  '204255221017214977': 3,
  '208993609255485441': 5,
  '382197942762602496': 4,
  '664606841409634324': 5,
  '779857709155090432': 4,
  '381084508276916225': 5,
  '794200859612545075': 4,
  '155149108183695360': 4,
  '438978127973318656': 3,
  '304028235082104843': 2,
  '174548757151481856': 1
}

// arrange items into a dictionary of lists
// keyFunc determines bin for each item
function bin(items, keyFunc) {
    const bins = new Map();
    for (const item of items) {
        const key = keyFunc(item);
        if (!bins.get(key)) bins.set(key,[]);
        bins.get(key).push(item);
    }
    return bins;
}

// create a list of lists of player tuples
// each top level list represents a medal position
function createPodium(points) {
    // bin players by score
    const bins = bin(Object.entries(points), ([player,score])=>score);
    const podium = [...bins.entries()];
    // sort medal positions by descending score
    podium.sort((a,b)=>b[0]-a[0]);
    // return only the players
    return podium.map(([score,players])=>players);
}

const MEDAL_NAMES = ['first_place', 'second_place', 'third_place'];
function medalName(index) {
    const medal = MEDAL_NAMES[index];
    if (medal) return `:${medal}: `;
    else return '';
}

const playerString = (medal,[name,score]) =>
    medalName(medal)+
    `<${name}> had ${score} point${score>1?'s':''}`;

const podiumString = podium =>
    podium.flatMap((players,medal)=>
        players.flatMap(
            player=>playerString(medal,player)
        ).concat(['']) // add extra line between
    ).join('\n');

const podium = createPodium(points);
console.log(podiumString(podium));

Answer 3

You can also use string templating. Something like this:

const points = {
    '204255221017214977': 3,
    '208993609255485441': 5,
    '382197942762602496': 4,
    '664606841409634324': 5,
    '779857709155090432': 4,
    '381084508276916225': 5,
    '794200859612545075': 4,
    '155149108183695360': 4,
    '438978127973318656': 3,
    '304028235082104843': 2,
    '174548757151481856': 1
};

const placeTemplates = {
    5: (userId, points) => `:first_place: ${userId} had ${points} points`,
    4: (userId, points) => `:secondt_place: ${userId} had ${points} points`,
    3: (userId, points) => `:third_place_place: ${userId} had ${points} points`,
    2: (userId, points) => `${userId} had ${points} points`, // DRY :-/
    1: (userId, points) => `${userId} had ${points} points` // DRY :-/
}

const aggregated = Object.entries(points).reduce((akku, userId_point) => {
    const [userId, point] = userId_point;
    if(!Array.isArray(akku[point])) {
        akku[point] = []
    }
    akku[point].push(placeTemplates[point](userId, point));
    return akku;
}, {});

const result = Object.keys(aggregated).reverse().reduce((akku, point) => akku.concat(aggregated[point]), []).join("\n");
document.getElementById('wrapper').innerText = result;

<div id="wrapper"></div>