Mdthbs

The task is to create two random teams from a list of players.

Given an example of players = ["John", "Mike", "Alice", "Bob"]

One example random team would be:

team1 = ["Mike", "Alice"];

team2 = ["John", "Bob"];

This is my solution but I'm not sure how efficient it is because random can create same number for a number of times.

The question to separate equally and randomly without repeating the same player.

using System;

using System.Collections.Generic;



public class Program

{



    public static void Main(string[] args)

    {

        Random rnd = new Random();

        var numbers = new List<int>();



        var ilkListe = new List<int>();



        var ikinciListe = new List<int>();



        numbers.Add(1);

        numbers.Add(2);

        numbers.Add(3);

        numbers.Add(4);

        numbers.Add(5);

        numbers.Add(6);

        numbers.Add(7);

        numbers.Add(8);

        numbers.Add(9);

        numbers.Add(10);

        Console.WriteLine("LIST 1: " + numbers.Count);



        do {

            int sayi = numbers[rnd.Next(numbers.Count)];

            if (!(ilkListe.Contains(sayi))) {

                Console.WriteLine(sayi);

                ilkListe.Add(sayi);

            }

        }



        while (ilkListe.Count < numbers.Count / 2);



        Console.WriteLine("Ikinci liste");



        do {

            int sayi = numbers[rnd.Next(numbers.Count)];

            if (!(ilkListe.Contains(sayi)) && !(ikinciListe.Contains(sayi))) {

                Console.WriteLine(sayi);

                ikinciListe.Add(sayi);

            }

        }



        while (ikinciListe.Count < numbers.Count / 2);

    }

}

Review

• Don't use the main entry point to implement an algorithm. Create a method instead.


• Think about how to allow this method to be usable for all kinds of types, not just integers. You have implemented the algorithm for integers, yet you show an example with strings.


• When creating an algorithm, surely you have made some unit tests, at least for the happy paths. Feel free to include them in the question.


• Use clear variable names, and stick to one written language.


• Don't mix algorithm logic with Console output.

A possible method signature could be:

public static IEnumerable<IEnumerable<T>> SplitRandom<T>(

    this IEnumerable<T> source, int targetCount)

{

    // .. algorithm implementation

}

Called as..

var names = new [] { "John", "Mike", "Alice", "Bob" };

var teams = names.SplitRandom(2);

Or as..

var numbers = Enumrable.Range(1, 10);

var numberGroups = numbers.SplitRandom(2);

using System;

using System.Collections.Generic;



public class Program

{



    public static void Main(string[] args)

    {

        List<string> players = new List<string>(new string[]{ "John", "Mike", "Kate", "Michael" });



        List<string> randomPlayers = ShuffleList<string>(players);



        List<string> team1 = randomPlayers.GetRange(0, (randomPlayers.Count - 1) / 2);

        List<string> team2 = randomPlayers.GetRange((randomPlayers.Count - 1) / 2, (randomPlayers.Count - 1));

    }



    public static List<E> ShuffleList<E>(List<E> inputList)

    {

     List<E> randomList = new List<E>();



     Random r = new Random();

     int randomIndex = 0;

     while (inputList.Count > 0)

     {

          randomIndex = r.Next(0, inputList.Count); //Choose a random object in the list

          randomList.Add(inputList[randomIndex]); //add it to the new, random list

          inputList.RemoveAt(randomIndex); //remove to avoid duplicates

     }



     return randomList; //return the new random list

    }

}

After a little search of "array shuffle keyword", rewrite the code.

While your new solution does look somewhat better than the original, I would suggest that constantly removing elements from the list can be quite expensive. I would suggest shuffling or randomly sorting the list then simply divvying up the players to the teams.

Also you're still putting the implementation in Main, instead of calling a method.

Instead of hardcoding everything for 2 teams I would be in favor of allowing any number of teams as long as it's an exact divisor of the number of players.

Instead of multidimensional lists, I would use a Dictionary where the Key is the name of the team and the players names are in a list.

It could look something like this:

public static Dictionary<int,List<T>> MakeTeams<T>(this List<T> playerList, int numTeams)

{



    int count = playerList.Count();

    if(count % numTeams != 0)

    {

        throw new ArgumentException("The number of players must be a multiple of numTeams to get even distribution of players");

    }

    var randomList = playerList.OrderBy(x => Guid.NewGuid()).ToList();

    var teams = (from int i in Enumerable.Range(0, count)

                 let item = randomList[i]

                 group item by (i % numTeams) into team

                 select team).ToDictionary(team => team.Key, team => team.ToList());



    return teams;

}

This simply uses a number for the team name. If you wanted to get fancier with it you could add a string array of team names to use instead:

public static Dictionary<string, List<T>> MakeTeams<T>(this List<T> playerList, int numTeams, string[] teamNames)

{

    if(teamNames.Length < numTeams)

    {

        throw new ArgumentException("The number of team names must be equal to or greater than numTeams");

    }

    int count = playerList.Count();

    if (count % numTeams != 0)

    {

        throw new ArgumentException("The number of players must be a multiple of numTeams to get even distribution of players");

    }

    var randomList = playerList.OrderBy(x => Guid.NewGuid()).ToList();

    var teams = (from int i in Enumerable.Range(0, count)

                 let item = randomList[i]

                 group item by (i % numTeams) into team

                 select team).ToDictionary(team => teamNames[team.Key], team => team.ToList());



    return teams;

}

All of dfhwze's points are spot-on, and the only thing I can think of to add to his type-signature is that if you're unfamiliar with all of the syntax he's using that's ok.
_{(If you want to learn the stuff in question, search for "inheritance" (or "interface"), "generics", "extension methods", and possibly "generators" or "yield".)}

Regarding your actual algorithm:

The big red flag that your current algorithm isn't good is that you have two nearly-identical loops. That they happen to be while loops is a small red flag, which you noticed indirectly when you noted that rnd.Next() can yield duplicates.

The algorithm you probably want is to first shuffle your source list, and then take sequential runs from it as needed.
Stack Exchange gives us usable implementations of both those steps. Only copy-paste them if you understand how they work and you want those exact methods lying around your code-base; most of the time it's better to re-write these things so that you're making the "details" decisions for yourself.

• 
  This answer gives us a nice implementation of The Fisher–Yates shuffle, which is efficient and appropriate.


• 
  This answer gives us tidy slicing. (What exactly happens if source.Count / (Double)size isn't an integer?)

There are some details that probably matter, like whether or not the original order of the elements needs to be preserved, or what should happen if the source list isn't evenly divisible into the number of buckets in question.

And once you've resolved all of that, since you asked about efficiency, we can return to dfhwze's function signature. Specifically, this IEnumerable<T> source is taken as an enumerable collection; the Fisher–Yates shuffle shuffles a list in place. It's very tempting to change the argument to this IList<T> source, so that if the calling context already has a list, we're can just use that one instead of calling source.ToList(). If you do it that way you'll have written a function that returns a computed value and modifies the contents of its argument; strongly consider not doing that.

Below is a slightly improved algorithm of the problem.
This uses one while loop as opposed to two.

using System;

using System.Collections.Generic;



class Program

{

    static List<String> AllPlayers = new List<String>();



    static List<string> Team1 = new List<string>();

    static List<string> Team2 = new List<string>();



    static void Main(string[] args)

    {



        AllPlayers.Add("A1");

        AllPlayers.Add("A2");

        AllPlayers.Add("A3");

        AllPlayers.Add("A4");

        AllPlayers.Add("A5");

        AllPlayers.Add("A6");





        AllotPlayers();





        Console.WriteLine("Team1");

        foreach (var p in Team1)

            Console.Write(" " + p + " ,");

        Console.WriteLine();

        Console.WriteLine("Team2");

        foreach (var p in Team2)

            Console.Write(" " + p + " ,");

    }



    static void AllotPlayers()

    {

        Random rnd = new Random();

        int count = 0;

        bool firstTeam = false;

        string player = string.Empty;

        int index = -1;

        do

        {

            index = rnd.Next(AllPlayers.Count);

            player = AllPlayers[index];

            if (player == string.Empty) continue;



            AllPlayers[index] = "";

            if (firstTeam) Team2.Add(player);

            else Team1.Add(player);

            firstTeam = !firstTeam;

            count++;

        } while (count < AllPlayers.Count);

    }

}