Generating Unique Colors

Question

I just wrote a small class that generates perfectly unique colors(or that is the plan anyway). I found the original index shifting snippet online, and was just wondering if it could be improved in anyway, or if any problems could arise from using it.

Here is the class which is used to create the unique colors.

public class UniqueColorGenerator
    {
        private int _colorIndex;
        public UniqueColorGenerator()
        {

        }

        public Color Next()
        {
            _colorIndex++;
            byte red = ( byte )( _colorIndex & 0x000000FF );
            byte green = ( byte )( ( _colorIndex & 0x0000FF00 ) >> 08 );
            byte blue = ( byte )( ( _colorIndex & 0x00FF0000 ) >> 16 );
            return Color.FromArgb( red , green , blue );
        }
    }

I am using this for "geometry picking" in OpenGL. It works beautifully, too. Here is a picture of 16,384 quads with their own unique color... or I guess I should say "unique shade of red and black".

When I first ran it, I could swear that there were obvious duplicates, but there aren't. Not only did the picking work flawlessly, but I also ran through the Colors and didn't get a single collision. Though I am still wondering if any problems could arise from how the generator is currently built—besides hitting integer max, which will also never happen.

Also, how can I randomize the colors more, so instead of getting different shades of red, I could receive more colors along the spectrum?

Answer 1

There's not much code here, but I think it can be pretty cleanly divided into two parts:

• Generate three bytes for a color from an integer
• Have a way of getting multiple integers which are guaranteed to be unique

Simple as your algorithm is, you're already running into limitations by coupling these two pieces of functionality together (which, by the way, makes this an excellent object lesson in the importance of the Single Responsibility Principle!)

Namely, in your question, all the colors are red, and in your answer, you're still getting colors following a completely deterministic numeric pattern. Depending on your offset, this pattern will be more or less obvious (it's not just low vs. high- I'm pretty sure there's an offset which would give you the first image with red turned to blue, for example).

So, let's see what we can do about this unwanted coupling.

---

Separating Responsibilities

As I said before, you have two pieces of functionality. The first is given by these lines:

byte red = ( byte )( _colorIndex & 0x000000FF );
byte green = ( byte )( ( _colorIndex & 0x0000FF00 ) >> 08 );
byte blue = ( byte )( ( _colorIndex & 0x00FF0000 ) >> 16 );
return Color.FromArgb( red , green , blue );

This makes a nice unit of functionality itself, with a single, simple purpose: Get a color from an integer index. So put the above in a method (say, GetColorFromIndex, with index as a parameter replacing _colorIndex).

The other ingredient is generating unique integers. This is what you really want to separate out into its own class, implementing an interface. That way, you can generate numbers however you want to get different patterns: increment by one, increment by an offset, use the numbers 1 to N shuffled randomly, whatever you can think of! By using polymorphism, instead of being limited to one way of generating numbers with some potential for parametrization (like offsetIncrement), you will be able to use whatever algorithms you want.

Fortunately, an interface already exists for exactly what you want: IEnumerable<int>. You can often think of an IEnumerable<T> as a collection to iterate over, but in some situations I find it more conceptually useful to think of it as a box that I can just keep asking for a new T (though it may eventually reach the end of its Ts). In this case, that's exactly what you want- a box you can just keep pulling integers out of.

So let's pull this together:

public class ColorGenerator
{
    private IEnumerator<int> _indexGenerator;
    public ColorGenerator(IEnumerable<int> indexGenerator)
    {
        _indexGenerator = indexGenerator.GetEnumerator();
    }

    public Color Next()
    {
        _indexGenerator.MoveNext();
        return GetColorFromIndex(_indexGenerator.Current);
    }

    private Color GetColorFromIndex(int index)
    {
        byte red = ( byte )( index & 0x000000FF );
        byte green = ( byte )( ( index & 0x0000FF00 ) >> 08 );
        byte blue = ( byte )( ( index & 0x00FF0000 ) >> 16 );
        return Color.FromArgb( red , green , blue );
    }
}

Note that now I've removed Unique from the name- it's now the responsibility of the passed in IEnumerable<int> to decide if it's unique or not.

I'll leave implementing IEnumerable<int> for your 'color offset' version as an exercise, it should be straightforward.

---

IEnumerable

Okay, the above is a bit ugly- lots of fiddling with enumerators, and unhandled error cases (what happens if you reach the end of a finite collection of integers?). But this is because we're missing another trick: ColorGenerator itself is a box you can pull Colors out of, so it should itself be an IEnumerable<Color>. The structure of just repeatedly asking for a "Next" value very closely matches how IEnumerator<T>is laid out. So that aforementioned ugliness is a code smell specific to C# hinting at this to us.

So, making that adjustment, we end up with:

public class ColorGenerator : IEnumerable<Color>
{
    private IEnumerable<int> _indexGenerator;
    public ColorGenerator(IEnumerable<int> indexGenerator)
    {
        _indexGenerator = indexGenerator;
    }

    public IEnumerator<Color> GetEnumerator()
    {
        foreach(var index in _indexGenerator)
        {
            yield return GetColorFromIndex(index);
        }
    }

    private Color GetColorFromIndex(int index)
    {
        byte red = ( byte )( index & 0x000000FF );
        byte green = ( byte )( ( index & 0x0000FF00 ) >> 08 );
        byte blue = ( byte )( ( index & 0x00FF0000 ) >> 16 );
        return Color.FromArgb( red , green , blue );
    }
}

Now, as a bonus from our refactoring we get all the power and expressiveness of LINQ extensions for our color generator. If we want 10 colors, instead of:

var colors = new List<Color>();
for(int i=0; i<10; i++)
{
    colors.Add(colorGenerator.Next());
}

We can do:

var colors = colorGenerator.Take(10);

Answer 2

You don't need the constructor, as it's empty.

You're taking red as the low bits and blue as the high bits, which is backwards.

You appear to be using System.Drawing and not System.Windows.Media, judging by the overloads. If so, there is an overload that takes an int:

public class UniqueColorGenerator
{
    private int _colorIndex;

    public Color Next()
    {
        return Color.FromArgb(++_colorIndex);
    }
}

You also wouldn't need to use &, as casting to a byte will cut off the upper bits.

return Color.FromArgb(
    (byte)(_colorIndex >> 24), 
    (byte)(_colorIndex >> 16), 
    (byte)(_colorIndex >> 8), 
    (byte)_colorIndex);

Answer 3

I just wanted to post my own little something. I won't be accepting this as the answer, but I just wanted to show a simple mockup using the suggestions that I had already recieved.

public class UniqueColorGenerator {
        private int _colorIndex;
        private readonly int _offsetIncrement;

        public UniqueColorGenerator() {
            this._offsetIncrement = 1;
        }

        public UniqueColorGenerator( uint offsetIncrement ) {
            this._offsetIncrement = ( int )offsetIncrement;
        }

        public Color Next() {
            return Color.FromArgb( _colorIndex += _offsetIncrement );
        }
    }

That class does the exact same thing as my original example, but is a lot simpler, and even adds room for a bit of uniqueness. The higher number that you add in the constructor, the more crazy the final pattern becomes; here is a quick example of what happens if you feed "2834783" into the constructor:

And yes, all of the colors are still unique. The only issue that I can see from this new code is an issue that would arise if the user put in a ridiculously high number.