Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Avoiding Duplication for Random.GetItems #102229

Open
mertmtn opened this issue May 14, 2024 · 7 comments
Open

[API Proposal]: Avoiding Duplication for Random.GetItems #102229

mertmtn opened this issue May 14, 2024 · 7 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime untriaged New issue has not been triaged by the area owner

Comments

@mertmtn
Copy link

mertmtn commented May 14, 2024

Background and motivation

The Random.Shared.GetItems method will not get items in a mutually exclusive way.
When retrieves items randomly, resulting random collection can include duplicate entries.

Therefore, I use dictionary to store values as key and boolean flag value as value in GetItems method.
In my idea, I guaranteed the distinct and randomly selection using the dictionary,

If randomly selected value has already taken, it will not get items again.

using GoogleMapPoint = (string, double);
namespace Dotnet8NewFeatures
{
    public class Person(string name, string surname, int age, GoogleMapPoint point)
    {
        public string Name => name;
        public string Surname => surname;
        public int Age => age;
        public GoogleMapPoint Point => point;
    }
}

List<Person> persons = new List<Person>();

persons.AddRange([
                new Person("John", "Doe", 20,("Newyork",123)),
                new Person("Jane", "Saw", 30,("LA",456)),
                new Person("Tarzan", "Jungle", 40,("Seattle",666)),
                new Person("Mert", "Metin", 45,("IST",412)),
                new Person("Martin", "Fowler", 50,("MC",734)) ]);

var selectedPersonList = Random.Shared.GetItems(persons.ToArray(), 2);

// Getting the values out
foreach (var p in selectedPersonList )
   Console.WriteLine($"{p.Name}-{p.Surname}-{p.Age}-{p.Point}");

image

API Proposal

namespace System.Collections.Generic;

        public void GetItems<T>(ReadOnlySpan<T> choices, Span<T> destination)
        {
            if (choices.IsEmpty)
            {
                throw new ArgumentException(SR.Arg_EmptySpan, nameof(choices));
            } 
            ArgumentOutOfRangeException.ThrowIfNegative(length);
 
            //Dictionary value is defined for flag. Initially all values are set to false
            var choicesDictionary = choices.Distinct().ToDictionary(x => x, x => false);

            var i = 0;
            while (destination.Length != choicesDictionary.Count(x => x.Value))
            {
                 var element = choices[Next(choices.Length)];

                 if (!choicesDictionary[element])
                 {
                    destination[i] = element;
                    // If item is randomly selected, key value will be flagged as true  
                    choicesDictionary[element] = true;
                    i++;
                 }
            } 

            for (int i = 0; i < destination.Length; i++)
            {
                destination[i] = choices[Next(choices.Length)];
            }
        }

API Usage

//Get items  for vowels
string[] vowels = ["a", "e", "i", "o", "u"];
var selectedList = Random.Shared.GetItems(vowels, 3);

// Getting the values out
foreach (var v in vowels)
    Console.WriteLine(v);


//Get items for Person class
List<Person> persons = new List<Person>();

persons.AddRange([
                new Person("John", "Doe", 20,("Newyork",123)),
                new Person("Jane", "Saw", 30,("LA",456)),
                new Person("Tarzan", "Jungle", 40,("Seattle",666)),
                new Person("Mert", "Metin", 45,("IST",412)),
                new Person("Martin", "Fowler", 50,("MC",734)) ]);

var selectedPersonList = Random.Shared.GetItems(persons.ToArray(), 2);

// Getting the values out
foreach (var p in selectedPersonList )
   Console.WriteLine($"{p.Name}-{p.Surname}-{p.Age}-{p.Point}");

using GoogleMapPoint = (string, double);
namespace Dotnet8NewFeatures
{
    public class Person(string name, string surname, int age, GoogleMapPoint point)
    {
        public string Name => name;
        public string Surname => surname;
        public int Age => age;
        public GoogleMapPoint Point => point;
    }
}

Alternative Designs

Risks

Not identified but defined potencial exceptions using below code blocks

ArgumentException(SR.Arg_EmptySpan, nameof(choices));
ArgumentOutOfRangeException.ThrowIfNegative(length);

@mertmtn mertmtn added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 14, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 14, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 14, 2024
@Clockwork-Muse
Copy link
Contributor

The current implementation(s) don't rely on the objects having any sense of equality. Such an implementation would require either an appropriate constraint and/or some sort of key extractor.

(In addition to needing to be a unique method, since you can't break the existing uses of the method by substituting this implementation).

There are potentially better workarounds using Shuffle on a copy of the source and using a sliced Span on the result.

@mertmtn
Copy link
Author

mertmtn commented May 14, 2024

The current implementation(s) don't rely on the objects having any sense of equality. Such an implementation would require either an appropriate constraint and/or some sort of key extractor.

(In addition to needing to be a unique method, since you can't break the existing uses of the method by substituting this implementation).

There are potentially better workarounds using Shuffle on a copy of the source and using a sliced Span on the result.

Even if In my oppinion; unique random selection is necessary in the method. Shuffle or some linq methods could be better workarounds.

@vcsjones vcsjones added area-System.Runtime and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 15, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

@neon-sunset
Copy link
Contributor

This can be done in a fairly straightforward way with the current API.

var numbers = new[] { 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8, 9, 9 };
var distinct = numbers.Distinct().ToArray();
Random.Shared.Shuffle(distinct);

Console.WriteLine(string.Join(", ", distinct));

@GMPrakhar
Copy link

GMPrakhar commented May 15, 2024

The current implementation(s) don't rely on the objects having any sense of equality. Such an implementation would require either an appropriate constraint and/or some sort of key extractor.

But that's not correct, as an object by definition should be equal to itself.

I am more inclined towards the reasoning that we can ask for a greater number of items than the length of the source, in which case anyway there will be duplicates.

@Clockwork-Muse
Copy link
Contributor

This can be done in a fairly straightforward way with the current API.

var numbers = new[] { 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8, 9, 9 };
var distinct = numbers.Distinct().ToArray();
Random.Shared.Shuffle(distinct);

Console.WriteLine(string.Join(", ", distinct));

Requires that the type be equatable, which isn't guaranteed to be the case.

@Clockwork-Muse
Copy link
Contributor

The current implementation(s) don't rely on the objects having any sense of equality. Such an implementation would require either an appropriate constraint and/or some sort of key extractor.

But that's not correct, as an object by definition should be equal to itself.

I am more inclined towards the reasoning that we can ask for a greater number of items than the length of the source, in which case anyway there will be duplicates.

Yes, that too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

5 participants