2

I'm developing a theatre reservation software. I'm using Windows Forms, the seats is represented by a 2-dimensioned array. And I draw the buttons as following:

public void DrawSeats()
{
    // pnl_seats is a Panel
    pnl_seats.Controls.Clear();
    // Here I store all Buttons instance, to later add all buttons in one call (AddRange) to the Panel
    var btns = new List<Control>();
    // Suspend layout to avoid undesired Redraw/Refresh
    this.SuspendLayout();
    for (int y = 0; y < _seatZone.VerticalSize; y++)
    {
        for (int x = 0; x < _seatZone.HorizontalSize; x++)
        {
            // Check if this seat exists
            if (IsException(x, y))
                continue;
            // Construct the button with desired properties. SeatSize is a common value for every button
            var btn = new Button
            {
                Width = SeatSize, 
                Height = SeatSize,
                Left = (x * SeatSize),
                Top = (y * SeatSize),
                Text = y + "" + x,
                Tag = y + ";" + x, // When the button clicks, the purpose of this is to remember which seat this button is.
                Font = new Font(new FontFamily("Microsoft Sans Serif"), 6.5f)
            };

            // Check if it is already reserved
            if (ExistsReservation(x, y))
                btn.Enabled = false;
            else
                btn.Click += btn_seat_Click; // Add click event

            btns.Add(btn);
        }
    }
    // As said before, add all buttons in one call
    pnl_seats.Controls.AddRange(btns.ToArray());
    // Resume the layout
    this.ResumeLayout();
}

But already with a seat zone of 20 by 20 (400 buttons), it spent almost 1 minute to draw it, and in debug I checked that the lack of performance, is the instantiation of the buttons.

There is a way to make it faster? Perhaps disable all events during the instatiation or another lightweight Control that has the Click event too?

UPDATE: lbl was a test, the correct is btn, sorry.

UPDATE 2:

Here is the IsException and ExistsReservations methods:

private bool IsException(int x, int y)
{
    for (var k = 0; k < _seatsExceptions.GetLength(0); k++)
        if (_seatsExceptions[k, 0] == x && _seatsExceptions[k, 1] == y)
            return true;
    return false;
}

private bool ExistsReservation(int x, int y)
{
    for (var k = 0; k < _seatsReservations.GetLength(0); k++)
        if (_seatsReservations[k, 0] == x && _seatsReservations[k, 1] == y)
            return true;
    return false;
}
Cœur
  • 37,241
  • 25
  • 195
  • 267
  • 1
    What does `ExistsReservation` and `IsException` do exactly? – mafafu Jul 31 '14 at 12:40
  • `var btn = new Button` but `btns.Add(lbl);`. what is lbl. – fliedonion Jul 31 '14 at 12:41
  • I think that you really show us the code for IsException and ExistsReservation because building a grid of 20x20 buttons is not a big work also for a Winform app, – Steve Jul 31 '14 at 12:49
  • 2
    Try remove IsException and ExistsReservation calls to see if speed increase. – tezzo Jul 31 '14 at 12:51
  • I also suggest using this instead of suspend/resume layout: http://stackoverflow.com/questions/487661/how-do-i-suspend-painting-for-a-control-and-its-children – tezzo Jul 31 '14 at 12:55
  • I didn't check your code to the comma, but it look like a raster layout; consider using a datagridview instead! But as other have noted, 200 buttons will certainly not take more than a few seconds to create & draw so there is something else taking up time... – TaW Jul 31 '14 at 12:57
  • 2
    Looking at the two methods now, I could only recommend to test what @tezzo said. Remove the two calls and check the difference. If this is the problem then you should find a different data structure to keep the reservation and the exclusion. Some structure that allows a fast access to a random element like a dictionary where you have a key composed by a string like the TAG used in the button – Steve Jul 31 '14 at 13:20
  • 1
    Yup, Steve is right. Without those calls i can run your code in 1/10s. What is `_seatsReservations` ? How large is it for your tests? Why do you have to run through it instead of looking it up? – TaW Jul 31 '14 at 13:24

4 Answers4

3

Suppose that you change your arrays for reservations and exclusions to

public List<string> _seatsExceptions = new List<string>();
public List<string> _seatsReservations = new List<string>();

you add your exclusions and reservations in the list with something like

_seatsExceptions.Add("1;10");
_seatsExceptions.Add("4;19");
_seatsReservations.Add("2;5");
_seatsReservations.Add("5;5");

your checks for exclusions and reservations could be changed to

bool IsException(int x, int y)
{
    string key = x.ToString() + ";" + y.ToString();
    return _seatsExceptions.Contains(key);
}
bool ExistsReservation(int x, int y)
{
    string key = x.ToString() + ";" + y.ToString();
    return _seatsReservations.Contains(key);
}

of course I don't know if you are able to make this change or not in your program. However consider to change the search on your array sooner or later.

EDIT I have made some tests, and while a virtual grid of 20x20 buttons works acceptably well (31 millisecs 0.775ms on average), a bigger one slows down noticeably. At 200x50 the timing jumps to 10 seconds (1,0675 on average). So perhaps a different approach is needed. A bound DataGridView could be a simpler solution and will be relatively easy to handle.

Steve
  • 213,761
  • 22
  • 232
  • 286
  • Can you send me your DataGridView example? Thank you! – Gabriel Ferreira Rosalino Jul 31 '14 at 14:17
  • Sorry, but I haven't written any example about that. (it is a bit of work). However you could get a good starting point from this MSDN article http://msdn.microsoft.com/en-us/library/vstudio/5s3ce6k8(v=vs.100).aspx – Steve Jul 31 '14 at 14:22
3

I also won't use such a myriad of controls to implement such a thing. Instead you should maybe create your own UserControl, which will paint all the seats as images and reacts on a click event.

To make it a little easier for you i created such a simple UserControl, that will draw all the seats and reacts on a mouse click for changing of the state. Here it is:

public enum SeatState
{
    Empty,
    Selected,
    Full
}

public partial class Seats : UserControl
{
    private int _Columns;
    private int _Rows;
    private List<List<SeatState>> _SeatStates;

    public Seats()
    {
        InitializeComponent();
        this.DoubleBuffered = true;

        _SeatStates = new List<List<SeatState>>();
        _Rows = 40;
        _Columns = 40;
        ReDimSeatStates();

        MouseUp += OnMouseUp;
        Paint += OnPaint;
        Resize += OnResize;
    }

    public int Columns
    {
        get { return _Columns; }
        set
        {
            _Columns = Math.Min(1, value);
            ReDimSeatStates();
        }
    }

    public int Rows
    {
        get { return _Rows; }
        set
        {
            _Rows = Math.Min(1, value);
            ReDimSeatStates();
        }
    }

    private Image GetPictureForSeat(int row, int column)
    {
        var seatState = _SeatStates[row][column];

        switch (seatState)
        {
            case SeatState.Empty:
                return Properties.Resources.emptySeat;

            case SeatState.Selected:
                return Properties.Resources.choosenSeat;

            default:
            case SeatState.Full:
                return Properties.Resources.fullSeat;
        }
    }

    private void OnMouseUp(object sender, MouseEventArgs e)
    {
        var heightPerSeat = Height / (float)Rows;
        var widthPerSeat = Width / (float)Columns;

        var row = (int)(e.X / widthPerSeat);
        var column = (int)(e.Y / heightPerSeat);
        var seatState = _SeatStates[row][column];

        switch (seatState)
        {
            case SeatState.Empty:
                _SeatStates[row][column] = SeatState.Selected;
                break;

            case SeatState.Selected:
                _SeatStates[row][column] = SeatState.Empty;
                break;
        }

        Invalidate();
    }

    private void OnPaint(object sender, PaintEventArgs e)
    {
        var heightPerSeat = Height / (float)Rows;
        var widthPerSeat = Width / (float)Columns;

        e.Graphics.CompositingQuality = System.Drawing.Drawing2D.CompositingQuality.HighQuality;
        e.Graphics.InterpolationMode = System.Drawing.Drawing2D.InterpolationMode.HighQualityBicubic;
        e.Graphics.PixelOffsetMode = System.Drawing.Drawing2D.PixelOffsetMode.HighQuality;
        e.Graphics.SmoothingMode = System.Drawing.Drawing2D.SmoothingMode.HighQuality;

        for (int row = 0; row < Rows; row++)
        {
            for (int column = 0; column < Columns; column++)
            {
                var seatImage = GetPictureForSeat(row, column);
                e.Graphics.DrawImage(seatImage, row * widthPerSeat, column * heightPerSeat, widthPerSeat, heightPerSeat);
            }
        }
    }

    private void OnResize(object sender, System.EventArgs e)
    {
        Invalidate();
    }

    private void ReDimSeatStates()
    {
        while (_SeatStates.Count < Rows)
            _SeatStates.Add(new List<SeatState>());

        if (_SeatStates.First().Count < Columns)
            foreach (var columnList in _SeatStates)
                while (columnList.Count < Columns)
                    columnList.Add(SeatState.Empty);

        while (_SeatStates.Count > Rows)
            _SeatStates.RemoveAt(_SeatStates.Count - 1);

        if (_SeatStates.First().Count > Columns)
            foreach (var columnList in _SeatStates)
                while (columnList.Count > Columns)
                    columnList.RemoveAt(columnList.Count - 1);
    }
}

This will currently draw forty rows and columns (so there are 800 seats) and you can click on each seat to change its state.

Here are the images i used:

  • EmtpySeat: emptySeat
  • ChoosenSeat: choosenSeat
  • FullSeat: fullSeat

If you anchor this control and resize it or you click on a seat to change its state there can be some minor lacking for the repainting if you further increase the number of rows or columns, but that is still somewhere far below one second. If this still hurts you, you have to improve the paint method and maybe check the ClipRectangle property of the paint event and only paint the really needed parts, but that's another story.

Oliver
  • 43,366
  • 8
  • 94
  • 151
1

Rather than using actual button controls, just draw the image of the seats then when the user clicks on a seat translate the mouse X,Y coordinates to determine which seat was clicked. This will be more efficient. Of course, the drawback is that you have to write the method to translate x,y coordinates to a seat, but that really isn't that difficult.

Bill W
  • 1,428
  • 1
  • 20
  • 29
  • I considered this option, but I find it difficult because of the interaction. But it still an option. I'll try some other optimizations and if not works, will do that. Thank you! – Gabriel Ferreira Rosalino Jul 31 '14 at 13:14
0

EDIT; it has been pointed out to me this will not work in Windows Forms!

Well, you are Sequentially working through it. if one iteration costs 1 sec, the full process will take 400*1 in time.

Perhaps you should try and make a collection of your objects, and process it 'parallel'.

try the .Net framework (4 and above) 'parallel foreach' method: http://msdn.microsoft.com/en-s/library/system.threading.tasks.parallel.foreach(v=vs.110).aspx

edit: so, if you have a list buttonnames, you can say

buttonNames.ForEach(x=>CreateButton(x));

while your CreateButton() method is as following:

private Button CreateButton(string nameOfButton) { Button b = new Button(); b.Text = nameOfButton; //do whatever you want... return b; }

Samjongenelen
  • 401
  • 4
  • 18
  • You can't do that. The thread that creates the control owns it and will not let any other thread mess it up. In a normal solution, this could have worked, but not in Windows Forms. – gretro Jul 31 '14 at 13:01