HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Winforms Circular Checkbox

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
circularcheckboxwinforms

Problem

So here is my next creation, a circular checkbox. This is my third custom control, so I'm still learning, but have the basics just about down. Tips and suggestions are most appreciated:

```
class CircularCheckbox : Control
{
private Color borderColor;
public Color BorderColor
{
get { return borderColor; }
set { borderColor = value; }
}

private Color background;
public Color Background
{
get { return background; }
set { background = value; }
}

private Color tickColor;
public Color TickColor
{
get { return tickColor; }
set { tickColor = value; }
}

private bool isChecked;
public bool IsChecked
{
get { return isChecked; }
set { isChecked = value; }
}

public CircularCheckbox(bool IsChecked)
{
DoubleBuffered = true;
SetStyle(ControlStyles.SupportsTransparentBackColor, true);
this.BackColor = Color.Transparent;

this.IsChecked = IsChecked;
Background = Color.White;
BorderColor = Color.Black;
TickColor = Color.Green;
}

public CircularCheckbox()
: this(false)
{ }

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

e.Graphics.SmoothingMode = System.Drawing.Drawing2D.SmoothingMode.AntiAlias;
Rectangle rc = this.ClientRectangle;
Graphics g = e.Graphics;
StringFormat sf = new StringFormat();
Font f = new Font("Arial", (float)rc.Height * 0.5f, FontStyle.Bold, GraphicsUnit.Pixel);

sf.Alignment = StringAlignment.Center;
sf.LineAlignment = StringAlignment.Center;

g.FillEllipse(new SolidBrush(Background), rc.Left + 1.5f, rc.Top + 1.5f, rc.Width - 4.0f, rc.Height - 4.0f);
g.DrawEllipse(new Pen(new SolidBrush(BorderColor), 3.0f), rc.Left + 1.5f, rc.Top + 1.5f, rc.Width - 4.0f, rc.Height - 4.0f);

if (IsChecked)
g.DrawString("\u2713", f, new Soli

Solution

-
you should use auto properties where you don't need any validation. So all of your properties should use them like instead of

private bool isChecked;
public bool IsChecked
{
    get { return isChecked; }
    set { isChecked = value; }
}


you would use

public bool IsChecked { get; set; }


-
this example of really unnecessary tenary expression

IsChecked = IsChecked ? false : true;


should be replaced by

IsChecked = !IsChecked;


-
using braces {} for single if statement also would make your code less errorprone

  • you should extract your magic numbers into some meaningful aka good named constants ( 3.0f , 1.5f etc)



  • you should extract "\u2713" to some meaningful constant



  • you should extract new SolidBrush(tickColor) to a field, as the brush itself won't change.

Code Snippets

private bool isChecked;
public bool IsChecked
{
    get { return isChecked; }
    set { isChecked = value; }
}
public bool IsChecked { get; set; }
IsChecked = IsChecked ? false : true;
IsChecked = !IsChecked;

Context

StackExchange Code Review Q#73907, answer score: 12

Revisions (0)

No revisions yet.