Design of colour fading for WinForms controls effect

I wrote this little piece of code to linearly interpolate between a winforms control's background colour and any arbirtrarly chosen colour.

I don't like the way I wrote this piece and I was wondering if any of you would be interested in suggesting improvements.

Some improvements/thoughts from the top of my head:

• If the user calls the method while a previous iteration is still ongoing, the background colour will not be predictable. This is not intended. I would require some form of if(!colourChanging.IsActive)... and so on. Any ideas on how to implement that?

• Do I need to dispose the task? No, right? The task will take a threadpool thread, release all of its resource at its end and return the thread back to the pool?

• Is this a stupid approach to begin with?

• How do I get the complementary colour of Color _eventColour? I think it might make sense to change the foreground color property to maintain readability.

• Is this approach scalable to lets say 100 controls - to be used in an event driven environment?

```
private void ChangeControlColour(Control _activeControl, Color _eventColour)
{
int[] _rgbEventColours = new int[3] { _eventColour.R, _eventColour.G, _eventColour.B };
int[] _rgbOriginalColours = new int[3] { _activeControl.BackColor.R, _activeControl.BackColor.G, _activeControl.BackColor.B };
int[] _rgbIntervals = new int[3];
int[] _rgbCurrentValues = _rgbEventColours;
int _intervals = 20;

/ linear steps between each fading interval /
for (int i = 0; i
{
System.Threading.Thread.Sleep(500);

Color _fadingColour;
for (int i = 0; i <= _intervals; i++)
{
_fadingColour = Color.FromArgb(_rgbCurrentValues[0], _rgbCurrentValues[1], _rgbCurrentValues[2]);

if (_activeControl.InvokeRequired)
_activeControl.Invoke((MethodInvoker)delegate {_activeControl.BackColor = _fadingColour;});
else
_activeControl.BackColor = _

-
The standard naming convention in C# for local variables and parameters is camelCase. An underscore prefix is usually used for private class fields.

-
You repeat the code so the the background color three times - this should be extracted into a separate method.

-
This will perform an integer division:

_rgbIntervals[i] = (_rgbOriginalColours[i] - _rgbEventColours[i]) / _intervals;

which means if the difference between two color part is less than _intervals the calculated step is 0. Also you will get quite big accumulated errors due to the rounding. For example if you start with the color (30, 30, 30) and want it to fade to (60, 60, 60) then you will end up only fading to (50, 50, 50) because 60 - 30 = 30 and 30 / 20 = 1.5 but the 1.5 will be truncated to 1 due to integer division.

You do set the final color to the target color but it would be smoother to calculate the step sizes as floats.

-
You keep track of the various part and intermediate values in a bunch of arrays which do not have any direct relationship. It's also not very re-usable this way. I would encapsulate it in it's own class.

So the refactored code could look like this:

Class to encapsulate the fading between two colors:

public class ColorFader
{
    private readonly Color _From;
    private readonly Color _To;

    private readonly double _StepR;
    private readonly double _StepG;
    private readonly double _StepB;

    private readonly uint _Steps;

    public ColorFader(Color from, Color to, uint steps)
    {
        if (steps == 0)
            throw new ArgumentException("steps must be a positive number");

        _From = from;
        _To = to;
        _Steps = steps;

        _StepR = (double)(_To.R - _From.R) / _Steps;
        _StepG = (double)(_To.G - _From.G) / _Steps;
        _StepB = (double)(_To.B - _From.B) / _Steps;
    }

    public IEnumerable Fade()
    {
        for (uint i = 0; i < _Steps; ++i)
        {
            yield return Color.FromArgb((int)(_From.R + i * _StepR), (int)(_From.G + i * _StepG), (int)(_From.B + i * _StepB));
        }
        yield return _To; // make sure we always return the exact target color last
    }
}

And the refactored method in your form:

private void ChangeControlColour(Control activeControl,  Color eventColour) 
{
    int intervals = 20;

    var colorFader = new ColorFader(eventColour, activeControl.BackColor, intervals);

    SetControlBackColor(eventColor);

    /*  LinearFading Process isolated in a seperate Task to avoid blocking UI   */
    Task t = Task.Factory.StartNew(() =>
    {
        System.Threading.Thread.Sleep(500);
        foreach (var color in colorFader.Fade())
        {
            SetControlBackColor(color);
            System.Threading.Thread.Sleep(50);
        }
    });
}

private void SetControlBackColor(Color color)
{
    if (_activeControl.InvokeRequired)
        _activeControl.Invoke((MethodInvoker)delegate { _activeControl.BackColor = color; });
    else
        _activeControl.BackColor = color;
}

StackExchange Code Review Q#62840, answer score: 5