Jesse posted a comment with a unique approach: Make a class that's good at laying out buttons on the bottom-right of a form.  I could definitely imagine adding a Help button, for example, which would make this generality helpful.

As for clarity, the downside is that the class itself is a bit more complexity to understand.  On the upside, it's general in a way that lets you separate understanding of the implementation and the consumer.  I think it's a win - you encapsulate the complexity & provide a pleasant programming model.

Here’s Jesse’s code:

 

    void CornerAlignControls(int spacing, Control[] controls)

    {

        if (controls.Length == 0) return;

 

        Size vertSpacing = new Size(spacing, 0);

        Size horzSpacing = new Size(0, spacing);

 

        Point lastLocation = new Point(parent.ClientSize - (vertSpacing + GetHeight(controls[0])));

        for (int i = controls.Length - 1; i >= 0; i--)

        {

            lastLocation = (controls[i].Location = lastLocation - (horzSpacing + GetWidth(controls[i])));

        }

    }

    Size GetWidth(Control c)

    {

        return new Size(c.Width, 0);

    }

 

    Size GetHeight(Control c)

    {

        return new Size(0, c.Height);

    }

 

One of the really interesting things that happens in refactoring is that after reading the refactored code you see something else worth refactoring.

 

In this case, I think that my previous recommendation to prefer Size/Point object to separate X & Y values is no longer a good idea.  The reason is that that Y value is held constant over the algorithm, while the X is manipulated repeatedly.  Indicators (smells) include the GetWidth, GetHeight, vertSpacing, and horzSpacing items.

 

With that in mind, I’ve refactored Jesse’s code to deal with X & Y separately:

 

    void CornerAlignControls(int spacing, Control[] controls)

    {

 

        int lastXLocation = Parent.ClientSize.Width;

        int YLocation = Parent.ClientSize.Height - spacing;

 

        for (int i = controls.Length - 1; i >= 0; i--)

        {

            lastXLocation -= controls[i].Width - spacing;

            controls[i].Location = new Point(lastXLocation, YLocation);

        }

    }

 

I also get to remove the check for controls.Length, further simplifying the code.  (if it turns out to be a perf hit because we call this routine in a tight loop with no controls, then we’ll add the check back in).

 

I’m not 100% sure it’s an improvement.  Maybe if we keep the location pair in a Point, but manipulate the X directly:

 

    void CornerAlignControls(int spacing, Control[] controls)

    {

        Point lastLocation = new Point(Parent.ClientSize.Width, Parent.ClientSize.Height - spacing);

 

        for (int i = controls.Length - 1; i >= 0; i--)

        {

            lastLocation.X -= controls[i].Width - spacing;

            controls[i].Location = lastLocation;

        }

    }

 

That does seem better to me. 

 

One last smell: the reverse iteration.  I find that when I write reverse iteration, I often get it wrong.  It seems simple, but I still screw it up.  Anyway, I like using foreach.

 

    void CornerAlignControls(int spacing, Control[] controls)

    {

        List<Control> controlsReversedList = new List<Control>(controls).Reverse;

 

        CornerAlignControls_Reversed(spacing, controlsReversedList.ToArray());

    }

 

    void CornerAlignControls_Reversed(int spacing, Control[] controls)

    {

        Point lastLocation = new Point(Parent.ClientSize.Width, Parent.ClientSize.Height - spacing);

 

        foreach (Control control in controls)

        {

            lastLocation.X -= control.Width - spacing;

            control.Location = lastLocation;

        }

    }

 

(I cheated a little: I used Generics that most of you don’t have access to.  But you will.)

 

What do you think?