6
\$\begingroup\$

I wanted to do a very little project of a team scrambler. There are two teams of 5 players. Each player has a unique color (in his team) from a limited pool. You can use 2 actions :

  • Scramble the teams which mix all players between the two teams
  • Request new color for a team which reassign new color for each player of this team

At first, I tried to place players inside an array that I shuffled but since VueJs cannot detect a change on array reassignment natively it ends complicating the code way too much.

So I end up with this solution (Sandbox for better readability) where I added a position property in my players :

<template>
  <div class="container mx-auto">
    <!-- header -->
    <div class="my-4 flex flex-row justify-center space-x-4">
      <button class="btn btn-red" @click="scrambleTeam1Colors">Color 1</button>
      <button class="btn btn-red" @click="scrambleTeams">SCRAMBLE</button>
      <button class="btn btn-red" @click="scrambleTeam2Colors">Color 2</button>
    </div>
    <!-- teams -->
    <div class="my-2 flex flex-row justify-between">
      <!-- team 1 -->
      <div class="w-2/5 flex flex-col space-y-px">
        <div class="flex att-title items-center">
          <h3 class="flex-grow text-center">Team 1</h3>
        </div>
        <Player v-for="player in team1" :key="player.id" :player="player"/>
      </div>
      <div class="w-1/5"/>
      <!-- team 2 -->
      <div class="w-2/5 flex flex-col space-y-px">
        <div class="flex def-title items-center">
          <h3 class="flex-grow text-center">Team 2</h3>
        </div>
        <Player v-for="player in team2" :key="player.id" :player="player"/>
      </div>
    </div>
  </div>
</template>

<script>
import Player from "@/components/Player.vue";

export default {
  components: {
    Player
  },
  data() {
    return {
      player1: {
        id: 1,
        name: "Player 1",
        color: "red",
        position: 0
      },
      player2: {
        id: 2,
        name: "Player 2",
        color: "blue",
        position: 1
      },
      player3: {
        id: 3,
        name: "Player 3",
        color: "green",
        position: 2
      },
      player4: {
        id: 4,
        name: "Player 4",
        color: "orange",
        position: 3
      },
      player5: {
        id: 5,
        name: "Player 5",
        color: "indigo",
        position: 4
      },
      player6: {
        id: 6,
        name: "Player 6",
        color: "red",
        position: 5
      },
      player7: {
        id: 7,
        name: "Player 7",
        color: "blue",
        position: 6
      },
      player8: {
        id: 8,
        name: "Player 8",
        color: "green",
        position: 7
      },
      player9: {
        id: 9,
        name: "Player 9",
        color: "orange",
        position: 8
      },
      player10: {
        id: 10,
        name: "Player 10",
        color: "indigo",
        position: 9
      },
      colors: [
        "red",
        "blue",
        "green",
        "orange",
        "indigo",
        "yellow",
        "teal",
        "purple",
        "pink"
      ]
    };
  },
  computed: {
    players() {
      return [
        this.player1,
        this.player2,
        this.player3,
        this.player4,
        this.player5,
        this.player6,
        this.player7,
        this.player8,
        this.player9,
        this.player10
      ];
    },
    team1() {
      return this.players.filter(player => player.position < 5);
    },
    team2() {
      return this.players.filter(player => player.position > 4);
    }
  },
  methods: {
    scrambleTeams() {
      let positionsToAssign = Array.from(Array(10).keys());
      positionsToAssign = this.shuffle(positionsToAssign);
      this.players.forEach(
        player => (player.position = positionsToAssign.pop())
      );
      this.scrambleTeam1Colors();
      this.scrambleTeam2Colors();
    },
    scrambleTeam1Colors() {
      let colorsToAssign = this.shuffle([...this.colors]);
      this.team1.forEach(player => (player.color = colorsToAssign.pop()));
    },
    scrambleTeam2Colors() {
      let colorsToAssign = this.shuffle([...this.colors]);
      this.team2.forEach(player => (player.color = colorsToAssign.pop()));
    },
    shuffle(array) {
      let currentIndex = array.length;
      let temporaryValue;
      let randomIndex;

      // While there remain elements to shuffle...
      while (currentIndex !== 0) {
        // Pick a remaining element...
        randomIndex = Math.floor(Math.random() * currentIndex);
        currentIndex -= 1;

        // And swap it with the current element.
        temporaryValue = array[currentIndex];
        array[currentIndex] = array[randomIndex];
        array[randomIndex] = temporaryValue;
      }

      return array;
    }
  }
};
</script>

<style lang="postcss" scoped>
.att-title {
  @apply font-bold bg-red-400 text-white;
}

.def-title {
  @apply font-bold bg-teal-400 text-white;
}

.btn {
  @apply font-bold py-2 px-4 rounded;
}

.btn-red {
  @apply bg-red-500 text-white;
}

.btn-red:hover {
  @apply bg-red-700;
}
</style>

Since I am not really experienced in either javascript or vuejs I was wondering if I was missing a way easier way to manage my pool of players and the shuffling. Also, is there an alternative to declaring a computed for my array "players".

Note: The shuffle method is just an algorithm I copied so it is not part of the review

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Overall assessment

For a beginner this is a great start. The use of computed properties is nice- especially with the filtering of players into teams. The selection of colors are ascetically pleasing.

Suggestions

UI

Button Labels

The labels on the top three buttons are Color 1, SCRAMBLE and Color 2, yet it seems that all three lead to some sort of scramble effect. For the first and last buttons it might make sense to each label contain the word Scramble, or else abstract the word Scramble out into a parent container - e.g.

<link href="//unpkg.com/[email protected]/dist/tailwind.min.css" rel="stylesheet">
<fieldset>
  <legend>Scramble</legend>
  <button class="btn bg-red text-white rounded-full p-2">Team 1</button>
  <button class="btn bg-red text-white rounded-full p-2">Both Teams</button>
  <button class="btn bg-red text-white rounded-full p-2">Team 2</button>
</fieldset>

Spacing between player squares and labels

It would be wise to add some spacing between the squares and the player labels:

screenshot with arrow

JavaScript / VueJS

use const as default

It is wise to use const for declaring variables to avoid accidental re-assignment (e.g. colorsToAssign in scrambleTeam1Colors() and scrambleTeam2Colors(). Then when re-assignment is deemed necessary use let - e.g. positionsToAssign in scrambleTeams.

Setting up data

The values setup in data are a bit redundant. A for loop could be used for that:

data() {
  const data = {
    players: [],
    colors
  };
  for (let i = 1; i <= 10; i++) {
    data.players.push({
      id: i + 1,
      name: "Player " + i,
      color: colors[(i - 1) % 5],
      position: i - 1
    });
  }
  return data;
},

where colors is moved out above the export default statement. This allows the players section of computed to be removed completely.

spreading items into an array

Instead of calling Array.from() - e.g.

let positionsToAssign = Array.from(Array(10).keys());

the spread syntax can be used for the same result, without the need to call a function:

let positionsToAssign = [...Array(10).keys()];

Swapping values

instead of this

temporaryValue = array[currentIndex];
array[currentIndex] = array[randomIndex];
array[randomIndex] = temporaryValue;

One could use Destructuring assignment to swap variables

[array[randomIndex], array[currentIndex]] = [array[currentIndex],  array[randomIndex]];

However it seems that is slower than other techniques even though the V8 blog claims "Once we unblocked escape analysis to eliminate all temporary allocation, array destructuring with a temporary array is as fast as a sequence of assignments."1. There is a "hack" suggested in this SO answer by showdev that appears to be the fastest method to swap variables:

array[randomIndex] = [array[currentIndex], (array[currentIndex] = array[randomIndex])][0];

This eliminates the need for ‘temporaryValue`.

Decrement operator

Decrement by one can be simplified using a decrement operator -- - e.g. from

currentIndex -= 1;

To:

currentIndex--;
\$\endgroup\$

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.