Clearest Code Challenge: My answer

  • [アーティクル]

Here’s my solution.

        const int spacing = 12;

        _buttonCancel.Location = new Point(this.ClientSize - _buttonCancel.Size - new Size(spacing, spacing));

        _buttonOK.Location = _buttonCancel.Location - new Size(spacing + _buttonCancel.Width, 0);

It’s pretty short.

I didn’t create any new types to solve the problem, but there is still an important OO lesson to be learned here. As much as possible, I tried to deal with higher order types. See that I used arithmetic on Point and Size types, instead of on X and Y coordinates, for the most part. The only exception is the “spacing + _buttonCancel.Width” expression in the last line. I could have created a pair of Size objects, I suppose:

        const int spacing = 12;

        _buttonCancel.Location = new Point(this.ClientSize - _buttonCancel.Size - new Size(spacing, spacing));

        _buttonOK.Location = _buttonCancel.Location - new Size(spacing, 0) - new Size(_buttonCancel.Width, 0);

I’m not sure whether that’s an improvement or not. Your thoughts?

My real solution (the one I checked in to our source control) has 2 differences from the above code:

1. I extracted 2 methods

2. I included some comments

Here is what I actually checked in:

        void HACK_ResetButtonLocations()

        {

       const int spacing = 12;

            HACK_PlaceCancelButton(spacing);

            HACK_PlaceOKButton(spacing);

            this.PerformLayout();

        }

        void HACK_PlaceCancelButton(int spacing)

        {

            // place the Cancel button from the bottom-right corner of the form

            ButtonCancel.Location = new Point(this.ClientSize - ButtonCancel.Size - new Size(spacing, spacing));

        }

        void HACK_PlaceOKButton(int spacing)

        {

            // place the OK button from the Cancel button

            ButtonOK.Location = ButtonCancel.Location - new Size(spacing + ButtonCancel.Width, 0);

        }

 

I have an unpleasant taste in my mouth, still. None of the solutions I’ve seen, including my own, seem clean & elegant. Maybe one of you will read this and think of something better?

Hopefully in a few days the bug that I’m working around will be fixed in the internal build, and I can go back to using anchoring.

Comments

  • Anonymous
    March 26, 2004
    In your solution:

    const int spacing = 12;

    _buttonCancel.Location = new Point(this.ClientSize - _buttonCancel.Size - new Size(spacing, spacing));

    _buttonOK.Location = _buttonCancel.Location - new Size(spacing + _buttonCancel.Width, 0);

    I think you mean _buttonOK.Width by the last _buttonCancel.Width.

    I would write the definition of _buttonCancel.Location as

    new Point(this.ClientSize) - _buttonCancel.Size - new Size(spacing, spacing);

    because this has parenthese nesting depth of 1 instead of 2.

    Actually, you can even simply it further to:

    new Point(-spacing, -spacing) + this.ClientSize - _buttonCancel.Size;

    But this simplification is more syntactic than conceptual. I don't know if it helps or confuses.

    Finally, "higher order types" means types that take other types as parameters. For example, List as in List< int > is a higher order type, because it takes a type as a parameter (e.g. int) and produces a type in return (e.g. List<int>). Therefore, Size and Point are not higher order types.
  • Anonymous
    March 26, 2004
    Zhanyong: good response.

    The bug you mention is real, but not noticed because both buttons have the same width!

    Maybe I would just cast:
    _buttonCancel.Location = (Point)this.ClientSize - _buttonCancel.Size - new Size(spacing, spacing);

    I guess Higher Order Types is not the right term, but what is? There's something significant about using a Size instead of a Width and a Height. What do you call it?
  • Anonymous
    March 26, 2004
    Hmm...JayBaz_MS, I see the effect of using Size -- the equations resolve correctly to the upper Left Point of each control. But is it a little confusing?? -- If one is thinking like the objects:

    ??? = The size of the client - the size of the button - the size of the margin

    Is that equal to the new size of the form or button? (??) -- what is that?-- what does that mean in a domain that has a form object, button object, and a margin object?

    Under the hood, the compiler is subtracting the width of the client, button, and margin (and the height of ...). And the result is the left edge and top left point of the button -- the button location...

    Derek LaZard
  • Anonymous
    March 26, 2004
    I don't like confusing, for sure. If it really is confusing, then I've done it wrong.

    However, there isn't much that's "under the hood" here, and certainly not by the compiler. We're using overloaded operators, which is a little opaque, but that's about it. Also, the overloading is done by the Point and Size classes, not by the compiler.

    Upon further consideration, I'm now convinced that turning this.ClientSize into a Point right away is the right thing to do - it makes the types along the way be semantically correct. (Does that make any sense?)
  • Anonymous
    March 26, 2004
    To Jay: I can see what you were trying to convey. I think it boils down to encapsulation: instead of manipulating the internals of a data structure, you want to treat it as abstract and manipulate it as a whole. Maybe ADT (abstract data types) is what you want?

    To Derek: using Size as opposed to X and Y is not confusing at all, if you have basic knowledge on vectors or complex numbers. Such topics can be found in any linear algebra or mathematical analysis text books. Once you grasp it, you will agree that reasoning about a 2-D entity rather than reasoning about its 2 dimensions separately is rather natural and clear.
  • Anonymous
    March 26, 2004
    Something like this would keep things pretty clean and allow for different widths and as many buttons as you want:

    Size GetWidth(Control c)
    {
    return new Size(c.Width, 0);
    }

    Size GetHeight(Control c)
    {
    return new Size(0, c.Height);
    }

    void CornerAlignControls(int spacing, Control[] controls)
    {
    if(controls.Length == 0) return;

    Size vertSpacing = new Size(spacing,0);
    Size horzSpacing = new Size(0,spacing);

    Size lastWidth = new Size(0,0);
    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 + lastWidth));
    lastWidth = GetWidth(controls[i]);
    }
    }

    // ...

    CornerAlignControls(12, new Control[] { _buttonOK, _buttonCancel });
  • Anonymous
    March 26, 2004
    Oops... that should probably be more along the lines of:

    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])));
    }
    }
  • Anonymous
    March 26, 2004
    The comment has been removed
  • Anonymous
    March 26, 2004
    JayBaz_MS: yes, maybe that's it: (Point) clientSize ...

    But what is

    Point - size - size ?

    I think this has to be defined as a Point/Location, because the result doesn't exist in 2D space. Consider the following:

    AreaSquare - AreaSquare = AreaSquare

    That makes domain sense because the subtraction of 2 area is another/new area -- maybe even a reduction in area/size...

    :))

    Derek LaZard