Winforms Mathematical Operator Label

Question

I wrote a custom class which (atm) displays the +, -, =, x, and ÷ operators inside a circle. Apart from the issue of some symbols not appearing in the exact center (everything except + looks a bit off) it works great. It uses an enum to select the symbol to make using the ÷ symbol easier.

public enum Operators { Plus = '+', Minus = '-', Divide = '÷', Multiply = 'x', Equal = '=' };

class MathematicalOperator : Control
{
    private Operators op;
    public Operators Operator
    {
        get { return op; }
        set
        {
            if (value == op) return;
            op = value;
            Invalidate();
        }
    }

    private Color background;
    public Color Background
    {
        get { return background; }
        set
        {
            if (value == background) return;
            background = value;
            Invalidate();
        }
    }

    private Color foreground;
    public Color Foreground
    {
        get { return foreground; }
        set
        {
            if (value == foreground) return;
            foreground = value;
            Invalidate();
        }
    }

    public MathematicalOperator(Operators op)
    {
        SetStyle(ControlStyles.SupportsTransparentBackColor, true);
        BackColor = Color.Transparent;
        TabStop = false;
        Operator = op;
        Foreground = Color.Black;
        Background = Color.White;
        Width = 35;
        Height = 35;
    }

    public MathematicalOperator()
        : this(Operators.Equal)
    { }

    protected override void OnPaint(PaintEventArgs e)
    {
        base.OnPaint(e);

        Graphics graphics = e.Graphics;
        Rectangle rc = this.ClientRectangle;
        graphics.SmoothingMode = System.Drawing.Drawing2D.SmoothingMode.AntiAlias;
        Font font = new Font("Arial", rc.Height * 0.65f, FontStyle.Bold, GraphicsUnit.Pixel);
        StringFormat sf = new StringFormat();
        sf.Alignment = StringAlignment.Center;
        sf.LineAlignment = StringAlignment.Center;

        using (var background = new SolidBrush(Background))
        using (var foreground = new Pen(Foreground, 2.0f))
        using (var text = new SolidBrush(Foreground))
        {
            graphics.FillEllipse(background, rc.Left + 1.5f, rc.Top + 1.5f, rc.Width - 4.0f, rc.Height - 4.0f);
            graphics.DrawEllipse(foreground, rc.Left + 1.5f, rc.Top + 1.5f, rc.Width - 4.0f, rc.Height - 4.0f);
            graphics.DrawString(((char)Operator).ToString(), font, text, rc, sf);
        }
    }
}

Answer 1

The OnPaint Method

I would refactor your OnPaint method a little to use more descriptive variable names, and store the new values added to the Rectangle in another variable, just to make things more maintainable. (And perhaps a little quicker, as well.)

Start with:

protected override void OnPaint(PaintEventArgs e)
{
    base.OnPaint(e);

    Graphics graphics = e.Graphics;
    Rectangle rc = this.ClientRectangle;
    graphics.SmoothingMode = System.Drawing.Drawing2D.SmoothingMode.AntiAlias;
    Font font = new Font("Arial", rc.Height * 0.65f, FontStyle.Bold, GraphicsUnit.Pixel);
    StringFormat sf = new StringFormat();
    sf.Alignment = StringAlignment.Center;
    sf.LineAlignment = StringAlignment.Center;

    using (var background = new SolidBrush(Background))
    using (var foreground = new Pen(Foreground, 2.0f))
    using (var text = new SolidBrush(Foreground))
    {
        graphics.FillEllipse(background, rc.Left + 1.5f, rc.Top + 1.5f, rc.Width - 4.0f, rc.Height - 4.0f);
        graphics.DrawEllipse(foreground, rc.Left + 1.5f, rc.Top + 1.5f, rc.Width - 4.0f, rc.Height - 4.0f);
        graphics.DrawString(((char)Operator).ToString(), font, text, rc, sf);
    }
}

And work towards:

protected override void OnPaint(PaintEventArgs e)
{
    base.OnPaint(e);

    Graphics graphics = e.Graphics;
    Rectangle controlBounds = this.ClientRectangle;
    graphics.SmoothingMode = System.Drawing.Drawing2D.SmoothingMode.AntiAlias;
    Font operatorFont = new Font("Arial", controlBounds.Height * 0.65f, FontStyle.Bold, GraphicsUnit.Pixel);
    StringFormat stringFormatter = new StringFormat();
    stringFormatter.Alignment = StringAlignment.Center;
    stringFormatter.LineAlignment = StringAlignment.Center;

    RectangleF outsideBounds = new RectangleF(controlBounds.Left + 1.5f, controlBounds.Top + 1.5f, controlBounds.Width - 4.0f, controlBounds.Height - 4.0f);

    using (var backgroundBrush = new SolidBrush(Background))
    using (var foregroundPen = new Pen(Foreground, 2.0f))
    using (var textBrush = new SolidBrush(Foreground))
    {
        graphics.FillEllipse(backgroundBrush, outsideBounds);
        graphics.DrawEllipse(foregroundPen, outsideBounds);
        graphics.DrawString(((char)Operator).ToString(), operatorFont, textBrush, controlBounds, stringFormatter);
    }
}

This should help maintainability in the future. Yes, things are much more verbosely named, but variable/parameter/method name lengths only affect compilation performance, not run-time.

Qualifying Names

I would also consider fully-qualifying the names of the member variables of the class. I.e. this.Operator instead of Operator. Yes, it makes it much more verbose again, but it's also clearly expressing exactly where things are coming from. The idea is to be able to skim the code and know it immediately. (Not that you can't do that with the original, but it does remove ambiguities.)

If we make that last change as well, we end up with:

protected override void OnPaint(PaintEventArgs e)
{
    base.OnPaint(e);

    Graphics graphics = e.Graphics;
    Rectangle controlBounds = this.ClientRectangle;
    graphics.SmoothingMode = System.Drawing.Drawing2D.SmoothingMode.AntiAlias;
    Font operatorFont = new Font("Arial", controlBounds.Height * 0.65f, FontStyle.Bold, GraphicsUnit.Pixel);
    StringFormat stringFormatter = new StringFormat();
    stringFormatter.Alignment = StringAlignment.Center;
    stringFormatter.LineAlignment = StringAlignment.Center;

    RectangleF outsideBounds = new RectangleF(controlBounds.Left + 1.5f, controlBounds.Top + 1.5f, controlBounds.Width - 4.0f, controlBounds.Height - 4.0f);

    using (var backgroundBrush = new SolidBrush(this.Background))
    using (var foregroundPen = new Pen(this.Foreground, 2.0f))
    using (var textBrush = new SolidBrush(this.Foreground))
    {
        graphics.FillEllipse(backgroundBrush, outsideBounds);
        graphics.DrawEllipse(foregroundPen, outsideBounds);
        graphics.DrawString(((char)this.Operator).ToString(), operatorFont, textBrush, controlBounds, stringFormatter);
    }
}

Comments on 'Magic Values'

You should also consider specifying what some of the floating values are set in the manner they are for via comments. I.e. on the using (var foreground = new Pen(Foreground, 2.0f)) line it would not hurt to add a comment indicating why the width of the Pen is 2.0f. This is merely to help remind yourself in the future for if (when) you come back to the code.

Hardcoded 'Magic Values'

You should also consider adding a custom RectangleF TextPadding property that defaults to new RectangleF(1.5f, 1.5f, 4.0f, 4.0f) to remove those hard-coded values. This will also increase extensibility and reuse-ability greatly for the future. (We want to reuse this class in another programme, don't we?)

You could then write an extension method to add two rectangles together or similar. This bit is up to you, but it could definitely ease maintainability and reuse-ability.

Field Naming

I also, personally, would not name the private Operators field op. Generally speaking, fields should be the camelCase variant of the property they back (operator in your case). Though, personally, I don't use that idea, and instead I use the backing field name as equivalent to the property name but prefixed with an underscore (_Operator in your case). Both these options are open, and generally increase meaningfulness.

---

All of these suggestions are mostly for consistency and conventions so that if you have to go back into the code two years from now, it's very easy to follow. These are based on my personal flavours of writing code, and are no way concrete.

Take them all as you would like. :)