using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Threading;
namespace TIG076_Lab4
{
public partial class ThreadForm : Form
{
private Painter rectanglePainter = null;
private Painter squarePainter = null;
private Painter circlePainter = null;
public ThreadForm()
{
InitializeComponent();
cmbSize.DataSource = PaintingConstants.multiplierSizes;
cmbSize.SelectedIndex = 0;
PaintingConstants.sizeMultiplier = Int32.Parse(cmbSize.SelectedValue.ToString());
//Trying to make things more random!!
Random r1 = new Random(DateTime.Now.Millisecond * 9);
Random r2 = new Random(DateTime.Now.Second * DateTime.Now.Millisecond);
Random r3 = new Random(r1.Next(1,70) * r2.Next(33, 80));
rectanglePainter = new RectanglePainter(drawingBoard.CreateGraphics(), r1);
squarePainter = new SquarePainter(drawingBoard.CreateGraphics(), r2);
circlePainter = new CirclePainter(drawingBoard.CreateGraphics(), r3);
}
private void btnThread1_Click(object sender, EventArgs e)
{
if (!rectanglePainter.IsBusy)
{
rectanglePainter.RunWorkerAsync();
}
else
{
rectanglePainter.StopExecution();
}
}
private void btnThread2_Click(object sender, EventArgs e)
{
if (!squarePainter.IsBusy)
{
squarePainter.RunWorkerAsync();
}
else
{
squarePainter.StopExecution();
}
}
private void btnThread3_Click(object sender, EventArgs e)
{
if (!circlePainter.IsBusy)
{
circlePainter.RunWorkerAsync();
}
else
{
circlePainter.StopExecution();
}
}
private void cmbSize_SelectedIndexChanged(object sender, EventArgs e)
{
PaintingConstants.sizeMultiplier = Int32.Parse(cmbSize.SelectedValue.ToString());
}
private void btnClear_Click(object sender, EventArgs e)
{
this.drawingBoard.Invalidate();
}
}
}
//Painter.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.ComponentModel;
using System.Drawing;
using System.Threading;
namespace TIG076_Lab4
{
public abstract class Painter: BackgroundWorker
{
protected const int PAINTING_THICKNESS = 2;
protected Graphics myGraphics;
protected Pen myPen;
private Random myRandomizer;
protected Boolean isRunning;
public Painter(Graphics graphics, Random randomizer)
{
this.myGraphics = graphics;
this.myPen = null;
this.myRandomizer = randomizer;
this.isRunning = false;
this.WorkerSupportsCancellation = true;
}
protected abstract void DrawShape(Point location);
private Point RandomizePoint()
{
int x = myRandomizer.Next(PaintingConstants.DRAWING_MARGIN, PaintingConstants.DRAWING_WIDTH);
int y = myRandomizer.Next(PaintingConstants.DRAWING_MARGIN, PaintingConstants.DRAWING_HEIGHT);
return new Point(x, y);
}
protected override void OnDoWork(DoWorkEventArgs e)
{
this.isRunning = true;
while (this.isRunning)
{
DrawShape(RandomizePoint());
Thread.Sleep(200);
}
}
public void StopExecution()
{
this.isRunning = false;
}
}
}
//CirclePainter.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Drawing;
namespace TIG076_Lab4
{
public class CirclePainter : Painter
{
public const int DIAMETER = 10;
public CirclePainter(Graphics graphics, Random randomizer)
: base(graphics, randomizer)
{
this.myPen = new Pen(Color.Green, PAINTING_THICKNESS);
}
protected override void DrawShape(Point location)
{
int multipliedDiameter = DIAMETER * PaintingConstants.sizeMultiplier;
myGraphics.DrawEllipse(myPen, new Rectangle(location, new Size(multipliedDiameter, multipliedDiameter)));
}
}
}
//RectangePainter.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Drawing;
namespace TIG076_Lab4
{
public class RectanglePainter : Painter
{
private const int HEIGHT = 10;
private const int WIDTH = 16;
public RectanglePainter(Graphics graphics, Random randomizer)
: base(graphics,randomizer)
{
this.myPen = new Pen(Color.Blue, PAINTING_THICKNESS);
}
protected override void DrawShape(Point location)
{
int multipliedHeight = HEIGHT * PaintingConstants.sizeMultiplier;
int multipliedWidth = WIDTH * PaintingConstants.sizeMultiplier;
myGraphics.DrawRectangle(myPen, new Rectangle(location, new Size(multipliedWidth, multipliedHeight)));
}
}
}
//SquarePainter.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Drawing;
namespace TIG076_Lab4
{
public class SquarePainter : Painter
{
private const int SIDE = 10;
public SquarePainter(Graphics graphics, Random randomizer)
: base(graphics, randomizer)
{
this.myPen = new Pen(Color.Red, PAINTING_THICKNESS);
}
protected override void DrawShape(Point location)
{
int multipliedSide = SIDE * PaintingConstants.sizeMultiplier;
myGraphics.DrawRectangle(myPen, new Rectangle(location, new Size(multipliedSide, multipliedSide)));
}
}
}
// PaintingConstants.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
namespace TIG076_Lab4
{
public class PaintingConstants
{
public const int DRAWING_MARGIN = 5;
public const int DRAWING_HEIGHT = 350;
public const int DRAWING_WIDTH = 600;
public static int sizeMultiplier;
public static int[] multiplierSizes = { 1,2,3,4,5};
public enum TrafficLightState
{
RED,
YELLOW,
GREEN
}
}
}
Refactorings
No refactoring yet !
runefs
May 8, 2010, May 08, 2010 21:10, permalink
TO answer your question, no it's not a good idea to inherit for the sake of functionality. Your painter is _not_ a background worker it _uses_ a background worker and hence the relationship between painter and backgroundworker shuold be aggregation not inheritance
sebastianlarsson.myopenid.com
May 8, 2010, May 08, 2010 21:25, permalink
runefs: Okay, so instead of inheriting Backgroundworker I should have an instance variable of type backgroundworker in the painting class?
Ants
May 11, 2010, May 11, 2010 20:36, permalink
Yes, prefer composition over inheritance, so have a BackgroundWorker instance variable in the Painting class. For your reading enjoyment:
http://blogs.msdn.com/steverowe/archive/2008/04/28/prefer-composition-over-inheritance.aspx
Some comments about the code above:
- C# Naming guidelines recommend using PascalCase for constants. Your use of ALLCAPS is a C/C++ style.
- Your Click handlers have very similar code. Consider calling a helper function and passing in the appropriate Painter subclass.
- TrafficLightState enum seems to be unused.
- PaintingConstants.sizeMultiplier isn't constant. It changes each time the combobox value changes.
- Did you consider the warning is the BackgroundWorker documentation: "You must be careful not to manipulate any user-interface objects in your DoWork event handler. Instead, communicate to the user interface through the ProgressChanged and RunWorkerCompleted events." I'm assuming drawingBoard is a Panel control and therefore a user-interface object.
- Read on the other public properties and methods of the BackgroundWorker class. I think that isRunning is equivalent to CancellationPending and can be set to true by CancelAsync().
- I'm not seeing Pens or Graphics objects being disposed in your Painter class or subclasses. Who disposes them?
sebastianlarsson.myopenid.com
May 13, 2010, May 13, 2010 19:00, permalink
Thank you for your time Ants. Each of your comment was very valuable and I'll rework my code accordingly when I find that I have time for it. TrafficLightState is used, but I did not include all of my code - I just included the parts which I was unsure of and that was inheriting the way I did. Thanks a bunch!
Hi guys,
I am currently reading Martin Fowler's "Refactoring: Improving the design of existing code" and I have tried to apply his techniques in my latest laboration from a university course. Is this bad or good design? Is the inheritance appropriate (e.g. inheriting from Backgroundworker in the first place)?
Btw, here is a link to my entire solution if somebody want to have a look:
http://www.filefactory.com/file/b18h7d5/n/Lab4_Lab5_SebastianLarsson.zip
Visual studio 2010
Thanks
Sebastian