Recursive (or iterative) solution for reducing data in an array of objects-CodePudding

Write a function that takes in a list of color war game results and returns the winner as defined:

a) The team with most wins b) If there is a tie, the color with the most wins among the subset of tied teams c) Eventually, if/when the set of tied colors can no longer be reduced, pick one of the remaining tied teams at random

Issues:

I've written code that works with various different data inputs, but I've been told that most of it is top-level and global-scoped with little of it being written as a function. I'm confused by this and need some direction on how to re-write this as a proper function (because I kind of thought I already did, though perhaps not elegantly)
In my code, I've reduced the data up to two times (as needed per the data set)-- but as the directions indicate, there may be the need to reduce the code more than two times in the case of ties, and I need to find a recursive or iterative solution for reducing the code. I've already spent a lot of time on finding a recursive solution and it's eluded me.

const winner = (teamObj) => {
  const tally = {}
  const tiedReduceData = []
  const secondTally = {}
  const tiedFinals=[]

  const pickTeam = (teamObj) => {
    const [[team1, score1], [team2, score2]] = Object.entries(teamObj)
     return score1 === score2 ?
     null : score1 > score2 ? team1 : team2
  }

  for(const teamObj of data) {
    const team = pickTeam(teamObj)
  
    if(team) {
      tally[team] = (tally[team] ?? 0)   1
    }
  }

  const getMaxArray = Object.keys(tally)
    .filter(x => { 
      return tally[x] == 
        Math.max.apply(null, Object.values(tally))
     })

  const compare = data.forEach( e => {
      if(Object.keys(e).every(i => getMaxArray.includes(i))){
        tiedReduceData.push(e);
      }
  })

  for (const teamObj of tiedReduceData) {
    const team = pickTeam(teamObj)

    if(team) {
      secondTally[team] = (secondTally[team] ?? 0)  1
    }
  }

  const getMaxArraySubset = Object.keys(secondTally)
    .filter(x => {
      return secondTally[x] ==
        Math.max.apply(null, Object.values(secondTally))
    })

  data.forEach( e => {
     if(Object.keys(e).every(i => getMaxArraySubset.includes(i))){
        tiedFinals.push(e);
     }
  })

  const randomWinner = getMaxArraySubset[Math.floor(Math.random() * getMaxArraySubset.length)]

  const allTied = "No Winner. All Tied."


  if (getMaxArray.length === 1){
    return getMaxArray
  } else if (getMaxArray.length === 0){
    return allTied
   } else if( getMaxArraySubset.length === 1 ){
     return getMaxArraySubset  
   } else {
   return randomWinner
   } 
}

console.log(winner(data))

const data = [ 
{magenta: 1, lime: 0},
{magenta: 1, berry: 0},
{berry: 1, magenta: 0},
{berry: 1, sienna: 0},
{sienna: 1, lime: 0},
{sienna: 1, magenta: 0},
{magenta: 0, lime: 1},
{magenta: 0, berry: 1},
{berry: 0, magenta: 1},
{berry: 0, sienna: 1},
{sienna: 0, lime: 1},
]

CodePudding user response：

In one sense, I think your grader is confused. The only global variables here are data and your winner function. That's entirely appropriate.

But there is a fair bit to clean up.

const winner = (teamObj) => {
  const tally = {}
  const tiedReduceData = []
  const secondTally = {}
  const tiedFinals=[]

This is probably what was meant. While these are not global, they serve a similar role as globals inside your function, data you initialize here and then manipulate elsewhere without passing them through from call to call.

  const pickTeam = (teamObj) => {
    const [[team1, score1], [team2, score2]] = Object.entries(teamObj)
     return score1 === score2 ?
     null : score1 > score2 ? team1 : team2
  }

This seems fine, except that you reuse the name teamObj -- the parameter to winner, and you use it to mean something different here. Might I suggest game, contest, or some such instead? It's also not a descriptive name as the parameter to winner. Perhaps match or games or tournament?

  for(const teamObj of data) {
    const team = pickTeam(teamObj)
  
    if(team) {
      tally[team] = (tally[team] ?? 0)   1
    }
  }

This is related to the point about the pseudo-globals. The earlier declaration of tally = {} and this for-loop would best be combined in a single reduce call.

  const getMaxArray = Object.keys(tally)
    .filter(x => { 
      return tally[x] == 
        Math.max.apply(null, Object.values(tally))
     })

Three things. First, you might want to keep a local variable, rather than recalculating the maximum as you test every contestant. Second the name getMaxArray sounds like an action, something that should be a function. winners, topTier, nextRoundContestants, something like that would be more clear.

Third, note that

Math.max.apply(null, Object.values(tally))

can better be written in modern JS as

Math .max (...Object .values (tally))

  const compare = data.forEach( e => {
      if(Object.keys(e).every(i => getMaxArray.includes(i))){
        tiedReduceData.push(e);
      }
  })

The name compare here is totally misleading. What you're doing is filtering the games to only include those where both contestants are among the top tiers.

Again, removing the pseudo-global, we could write something like

const tiedReduceData = data .filter (game => 
  Object .keys (game) .every (team => topTier .includes (team))
)

(I couldn't not change getMaxArray to topTier. That name is really bad!)

Skipping secondTally and maxArraySubset. These should not be needed if we do our recursion properly.

  data.forEach( e => {
     if(Object.keys(e).every(i => getMaxArraySubset.includes(i))){
        tiedFinals.push(e);
     }
  })

  const randomWinner = getMaxArraySubset[Math.floor(Math.random() * getMaxArraySubset.length)]

  const allTied = "No Winner. All Tied."

Here you calculate all the things you might return, even though you will only ever return one of them. It makes more sense to do such calculations only when you know that you will need them.

  if (getMaxArray.length === 1){
    return getMaxArray
  } else if (getMaxArray.length === 0){
    return allTied
   } else if( getMaxArraySubset.length === 1 ){
     return getMaxArraySubset  
   } else {
   return randomWinner
   } 
}

You have four return conditions here, all with different results. But the problem had three specifications. Are you sure your "allTied" scenario is possible?

In fact, in the problem specification, condition (b) does not really designate a winner, but points to a recursion. So it's time to talk about that recursion.

If we have several teams with the highest score, we want to call our function again, but only for the games that include those specific teams -- or that's the way I read the problem. The trouble is that we might not reduce our set of players by doing this, for instance, three games, A beats B, B beats C, and C beats A, each with one win and one loss. That's the case where we want to choose a random winner. If we have reduced the number of players ('lime' has two wins, 'magenta', 'sienna', and 'berry' each have three, so we try again without 'lime') then we make a recursive call. Here you'll need to filter the games in the input to exclude those with players not in the top tier. The code above shows plenty of ability to do this sort of manipulation, so I leave that to you.

Before I looked at your code, I tried this on my own, and wrote a version that overlaps some with your notions, but also differs from them substantially. I wouldn't suggest using any of it except for inspiration, but here it is without any annotations or discussions:

const matchWinner = (match) => {
  const players = [...new Set (match .flatMap (
    game => Object .keys (game)
  ))]
  const scores = match .flatMap (Object .entries) .reduce (
    (a, [t, s]) => ((a [t] = (a[t]  || 0)   s), a), {}
  )
  const topScore = Math .max (...Object .values (scores))
  const topTier = Object .entries (scores) 
      .filter (([k, v]) => v == topScore) 
      .map (([k]) => k)

  return topTier .length == 1
    ? topTier [0]
  : topTier .length == players .length
    ? topTier [Math .floor (Math .random () * topTier .length)]
  : matchWinner (match .filter (game => Object .keys (game) .every (
      contestant => topTier .includes (contestant)
    )))
} 


const data = [{magenta: 1, lime: 0}, {magenta: 1, berry: 0}, {berry: 1, magenta: 0}, {berry: 1, sienna: 0}, {sienna: 1, lime: 0}, {sienna: 1, magenta: 0}, {magenta: 0, lime: 1}, {magenta: 0, berry: 1}, {berry: 0, magenta: 1}, {berry: 0, sienna: 1}, {sienna: 0, lime: 1}]

console .log (matchWinner (data)) 
//=> {"magenta":3,"lime":2,"berry":3,"sienna":3}  // First call
//   {"magenta":2,"berry":3,"sienna":2}           // Second call (recursively)
//   "berry"                                      // Sole winner

Update

With more data, I realized that the match results were not winner/loser but simply points per side. With that in mind, we can change how the scores are calculated inside the function. I choose the chess-style 1 point for a win, 0 points for a loss, and 1/2 point for a tie. It would be easy enough to change this to soccer-style 3 points for a win, 0 points for a loss, and 1 point for a tie, or really to any other system that assigns point in a similar manner. Here's a chess-style implementation. It should be able to simply drop into my function:

  const scores = match.reduce ((a, game) => {
    const [[name1, score1], [name2, score2]] = Object .entries (game)
    a [name1] = (a [name1] || 0)   (score1 > score2 ? 1 : score1 < score2 ? 0 : .5)
    a [name2] = (a [name2] || 0)   (score2 > score1 ? 1 : score2 < score1 ? 0 : .5)
    return a
  }, {})