Search code examples
javasolid-principlescode-duplication

Improving alghoritm to pair players - too much duplication code


I have got players. Players got preferred position. We have got 3 positions - DEFENDERS, ATTACKERS, BOTH (players who are good on both positions). I need to create teams of two. Pairing players should be like that: "An an attacker would be paired with defender or those who can play on both positions. Of course, those who are comfortable on both positions can play with each other in a team. Two defenders or two attackers are weaker than other combinations, so should be paired this way only when there are no other options."

I have written something like this. I know that this is naive implementation, but could not find better way to do this.

First I sort players by their positions so I have 3 lists. Then, using while loop I'm dealing with first attackers, then defenders, then attackers/defenders.

Is there any way to improve that? If not, I would like to know, how can I extract duplicated code into method.

package foostour.app;

import foostour.app.model.Player;
import foostour.app.model.Team;

import java.util.ArrayList;
import java.util.List;
import java.util.Random;

class TeamScheduler {

    private List<Team> teams = new ArrayList<>();

    private List<Player> defenders = new ArrayList<>();
    private List<Player> attackers = new ArrayList<>();
    private List<Player> attackersAndDefenders = new ArrayList<>();
    private int teamNumber = 0;

    List<Team> createTeams(List<Player> shuffledPlayers) {

        sortPlayersByPosition(shuffledPlayers);

        while (!attackers.isEmpty()) {
            Team team = new Team("Team " + (teamNumber + 1));
            team.setFirstPlayer(attackers.remove(new Random().nextInt(attackers.size())));
            if (!defenders.isEmpty()) {
                team.setSecondPlayer(defenders.remove(new Random().nextInt(defenders.size())));
            } else if (!attackersAndDefenders.isEmpty()) {
                team.setSecondPlayer(attackersAndDefenders.remove(new Random().nextInt(attackersAndDefenders.size())));
            } else if (!attackers.isEmpty())
                team.setSecondPlayer(attackers.remove(new Random().nextInt(attackers.size())));
            teams.add(team);
            teamNumber++;
        }


        while (!defenders.isEmpty()) {
            Team team = new Team("Team " + (teamNumber + 1));
            team.setFirstPlayer(defenders.remove(new Random().nextInt(defenders.size())));
            if (!attackers.isEmpty()) {
                team.setSecondPlayer(attackers.remove(new Random().nextInt(attackers.size())));
            } else if (!attackersAndDefenders.isEmpty()) {
                team.setSecondPlayer(attackersAndDefenders.remove(new Random().nextInt(attackersAndDefenders.size())));
            } else if (!defenders.isEmpty())
                team.setSecondPlayer(defenders.remove(new Random().nextInt(defenders.size())));
            teams.add(team);
            teamNumber++;
        }


        while (!attackersAndDefenders.isEmpty()) {
            Team team = new Team("Team " + (teamNumber + 1));
            team.setFirstPlayer(attackersAndDefenders.remove(new Random().nextInt(attackersAndDefenders.size())));
            if (!attackers.isEmpty()) {
                team.setSecondPlayer(attackers.remove(new Random().nextInt(attackers.size())));
            } else if (!defenders.isEmpty()) {
                team.setSecondPlayer(defenders.remove(new Random().nextInt(defenders.size())));
            } else if (!attackersAndDefenders.isEmpty()) {
                team.setSecondPlayer(attackersAndDefenders.remove(new Random().nextInt(attackersAndDefenders.size())));
            }
            teams.add(team);
            teamNumber++;
        }

        return teams;
    }

    private void sortPlayersByPosition(List<Player> players) {
        for (Player player : players) {
            if (player.getPreferredPosition() == PositionType.BOTH)
                attackersAndDefenders.add(player);
            else if (player.getPreferredPosition() == PositionType.ATTACK)
                attackers.add(player);
            else if (player.getPreferredPosition() == PositionType.DEFENSE)
                defenders.add(player);
        }
    }
}

Solution

  • You can simplify the three for-loops into one for-loop. In order to do this, you would end up with more inside of the for-loops. You would also have to change the condition in the for-loop in order to check all three lists.

    while (!both.isEmpty() || !attackers.isEmpty() || !defenders.isEmpty()) {
        if (!attackers.isEmpty()) {
            team.setFirstPlayer(attackers.remove(new Random().nextInt(attackers.size())));
        } else if (!both.isEmpty()) {
            team.setFirstPlayer(both.remove(new Random().nextInt(attackers.size())));
        } else if (!defenders.isEmpty()) {
            team.setFirstPlayer(defenders.remove(new Random().nextInt(attackers.size())));
        }
    
        if (!defenders.isEmpty()) {
            team.setFirstPlayer(defenders.remove(new Random().nextInt(attackers.size())));
        } else if (!both.isEmpty()) {
            team.setFirstPlayer(both.remove(new Random().nextInt(attackers.size())));
        } else if (!attackers.isEmpty()) {
            team.setFirstPlayer(attackers.remove(new Random().nextInt(attackers.size())));
        }
    }
    

    This will check the attackers first and put them on the team. If there are no attackers, it will put someone who can do both. The second set of if-statements puts a defender in the second position or someone who can do both in that position. Only in a case where there are significantly more attackers or defenders than the other will it put two defenders or two attackers on the same team.