Pick WPF Random Brush color

Question

I m trying to fill a Canvas with some random shapes( Ellipse and Rectangle). I need to pick a random color to fill the shapes with it. The problem that I have always the same random color for all the shape. When I debug my code I get random colors. Below is my Code :

public MainWindow()
{
    InitializeComponent();
    this.Loaded += delegate { InitializeSourceCanvas(); };
}

private void InitializeSourceCanvas()
{
    var rnd = new Random();
    const int height = 30, width = 30;
    for (int i = 0; i < 25; i++)
    {
        var shape = rnd.Next(10) > 4 ? (Shape)new Ellipse() : (Shape)new Rectangle();
        shape.Width = height;
        shape.Height = width;
        shape.Stroke = new SolidColorBrush(Colors.Black);
        shape.StrokeThickness = 1;
        shape.Fill = PickRandomBrush();
        Canvas.SetLeft(shape, rnd.NextDouble() * (_source.ActualWidth - width));
        Canvas.SetTop(shape, rnd.NextDouble() * (_source.ActualHeight - height));
        _source.Children.Add(shape);
    }
}

private Brush PickRandomBrush()
{
    Brush result = Brushes.Transparent;
    Random rnd = new Random();
    Type brushesType = typeof(Brushes);
    PropertyInfo[] properties = brushesType.GetProperties();
    int random = rnd.Next(properties.Length);
    result = (Brush)properties[random].GetValue(null, null);
    return result;
}

The resulted shape

I don't understand why this question is getting down-voted... I'm sure that anyone who writes C# long enough has run into this in one form or another — darkpbj, Dec 18 '14 at 15:19

darkpbj · Accepted Answer · 2015-11-16T22:08:57.883

4

Like Nuke suggests, new your Random object outside the for loop so it's only instantiated once, and then pass the same one in each time.

Random rnd = new Random();
for (int i = 0; i < 25; i++)
{
...
shape.Fill = PickRandomBrush(rnd);
...
}

Then edit your PickRandomBrush method to look like,

private Brush PickRandomBrush(Random rnd)
{
Brush result = Brushes.Transparent;
Type brushesType = typeof(Brushes);
PropertyInfo[] properties = brushesType.GetProperties();
int random = rnd.Next(properties.Length);
result = (Brush)properties[random].GetValue(null, null);
return result;
}

Edit: k, sraboy made a good point about running this code in quick succession--if called fast enough this will have the same seed. Here's a solution that's not elegant, but reasonably guaranteed to be unique:

Random rnd = new Random(Guid.NewGuid().ToString().GetHashCode());

(this would replace the first line)

edited Nov 16 '15 at 22:08

answered Dec 18 '14 at 15:16

darkpbj

2,892
4
22
32

1

Thks it works :), but i can't understand why it works fine in debug mode or when i add a message box the the resulted color – MRebai Dec 18 '14 at 15:21
1

Old topic, I know, but for anyone else that stumbles upon this: It works fine in debug mode, or with the addition of a MessageBox, because enough time is passing between the recurring recreation of rnd (via new Random()) that the seed value is different. In release mode, the new instantiations happen so quickly in succession that the seed value hasn't changed. Same seed means same list of pseudo-random numbers. – sraboy Aug 17 '15 at 06:07
1

@sraboy Oops, I didn't realize that you were answering the OP's question rather than commenting on my solution. Anyways, I made an edit based on your input to better guarantee randomness. – darkpbj Nov 16 '15 at 22:11

score 2 · Answer 2 · answered Dec 18 '14 at 15:11

From MSDN:

[..] different Random objects that are created in close succession by a call to the default constructor will have identical default seed values and, therefore, will produce identical sets of random numbers. This problem can be avoided by using a single Random object to generate all random numbers.

score 0 · Answer 3 · answered Dec 18 '14 at 15:08

0

The Random is because you are not binding the function to a timer, meaning it has the same conditions each time it is created.

One solution would be to tie the random to the Systemtime.Now Another solution would be to use the same Random instead of creating a new one.

answered Dec 18 '14 at 15:08

Johan

262
1
15

1

It's called 'seeding', not 'tieing'. And `Random` already uses the current time/tickcount as default seed, but that doesn't help when creating multiple instances in quick succession. – Pieter Witvoet Dec 18 '14 at 15:18

Pick WPF Random Brush color

3 Answers3