Arranging Buttons on a Process Enumerating Application

I had the following piece of code for populating a list of currently running processes in an IT Self-Help application :

var proclist = ProcessHelper.GetProcList();

var i = 0;
var j = 0;

foreach (var process in proclist)
{
    if (process.ButtonText == "") continue;
    var button = new Button();

    if (i  6) & (i  button.Visible = false;
    button.Click += (x, y) => process.Kill();

    button.Text = process.ButtonText;

    button.Image = process.ThumbNail;
    button.ImageAlign = ContentAlignment.MiddleLeft;

    processTab.Controls.Add(button);

    button.Width = 130;
    button.Height = 25;
    button.TextAlign = ContentAlignment.MiddleLeft; 
}

And today I realised, that if a user has more than 14 applications running, my logic falls apart on the third column and displays the items like this :

I managed to fix this with the following amendment (added a new variable for the third column) :

var proclist = ProcessHelper.GetProcList();

// For placing each button - need a var for each column 
var i = 0;
var j = 0;
var k = 0;

foreach (var process in proclist)
{
    if (process.ButtonText == "") continue;
    var button = new Button();

    if (i  6) & (i  button.Visible = false;
    button.Click += (x, y) => process.Kill();

    button.Text = process.ButtonText;

    button.Image = process.ThumbNail;
    button.ImageAlign = ContentAlignment.MiddleLeft;

    processTab.Controls.Add(button);

    button.Width = 130;
    button.Height = 25;
    button.TextAlign = ContentAlignment.MiddleLeft; 
}

And now the items appear correctly :

However, I couldn't help but think that this is not a very clean fix, because I just hard-coded in another column, and if a user every has 21 + applications open, the issue will occur again!

finally, I came up with this :

```
var i = 0;
var j = 0;
var column = 10;

foreach (var process in proclist)
{
if (process.ButtonText == "") continue;
var button = new Button();

if ((i > 0) & ((i % 7) == 0)

What will happen when there are more processes than you can fit buttons for on your form?

You're doing the hard work by hand, reinventing a wheel that's already there for you... and as you saw, it's not exactly scaling very well.

The wheel in question is called a FlowLayoutPanel, a winforms layout control that automatically handles control positioning for you, taking overflowing controls to a new row or column, depending on how you configured it - it even handles scrollbars for you, if you have more items than can fit into the panel.

That said, your wheel has a number of squeaky parts:

if (process.ButtonText == "") continue;

-
Comparing a string against an empty string, would usually be written like this:

if (string.IsNullOrWhiteSpace(process.ButtonText)
{
    continue;
}

-
In all versions/attempts of your layout code, you hard-coded the key values.

• 140 / the column width, should be computed from the width of a button

• 7 is arbitrary and should be allowed to change if/when the form is maximized or resized. Your screenshots crop the top part, but if you haven't removed the min/maximize buttons and allow resizing of the form, the user can easily wreck your layout; it doesn't take the button height into consideration either.

BUG?

It's a little beyond the scope of this review, but this line can potentially blow everything up:

button.Click += (x, y) => process.Kill();

There's a chance your application will not-so-graciously crash and burn if you bring up your app, then start task manager and kill a process, and then click the button for that process in your app.

The process to kill won't exist anymore, and if ProcessHelper.Kill() doesn't handle that situation, boom! Make sure your wrapper wraps the operation in a try..catch block.

StackExchange Code Review Q#128297, answer score: 3