Feels like too many If's!

Personally, I think the approach is incorrect (even with the improvement of the hash).

I believe you should separate the functionality of the picture box from the data. In other words, if the picture box toggles between red and black - wire that up separately from any data. Then expose a way to determine the state of the picture box.

I would probably create a user control that handles the picturebox toggling from red to black and expose an event (let's call it StateChanged) that gets sent when the user toggles the picturebox. In the event I would pass an Id that represents (an instance) of the control and also pass the toggle state (red or black).

Within the form I would create several custom controls and wire up the StateChange event in each of the controls to the same event handler. In the event handler I would change the state in the hash table based on the Id and state value.

This might sound like a bit of work, but in the end you'll get a cleaner implementation with a good separation of UI logic and data. The nice thing about this approach is that it lets you focus on just one aspect of the problem at a time rather than solving them both at once.

Quote:

---

Originally Posted by Arjay

Personally, I think the approach is incorrect (even with the improvement of the hash).

I believe you should separate the functionality of the picture box from the data. In other words, if the picture box toggles between red and black - wire that up separately from any data. Then expose a way to determine the state of the picture box.

I would probably create a user control that handles the picturebox toggling from red to black and expose an event (let's call it StateChanged) that gets sent when the user toggles the picturebox. In the event I would pass an Id that represents (an instance) of the control and also pass the toggle state (red or black).

Within the form I would create several custom controls and wire up the StateChange event in each of the controls to the same event handler. In the event handler I would change the state in the hash table based on the Id and state value.

This might sound like a bit of work, but in the end you'll get a cleaner implementation with a good separation of UI logic and data. The nice thing about this approach is that it lets you focus on just one aspect of the problem at a time rather than solving them both at once.

---

The logic here sounds like a very clean good idea. Sadly I don't think I fully understand how to implement it. I don't know what to do when you suggest to create a custom control which handles the picturebox toggle or how to expose the event.

Perhaps I do know how, but I don't understand the terminology. :/

Zip up a small sample project that toggles the red/black of the picture boxes and I'll modify it.

Quote:

---

Originally Posted by Arjay

Zip up a small sample project that toggles the red/black of the picture boxes and I'll modify it.

---

That is very kind Arjay.

I have uploaded it here: http://www.quickfilepost.com/downloa...f7d24b89a152c6

Thank you very much.

I can't open the Mobile 6 project type, if you want me to take a look at this, zip up a regular WinForms project.

Sorry about that.

I have created it as a windows form app here:
http://www.quickfilepost.com/downloa...98d93b39b7fbc7

Thank you :)

Demise, here you go. (btw, you can simply attach a zip file to a post directly by using the ManageAttachments button).

I've created a PictureBoxUC user control that simply toggles between Red/Black when the user clicks on the control. The user control also fires off a SelChange event that sends out Id and IsSelected event info.

I'm using a Guid Id as a representation of some entity (e.g. an industrial Machine). I set the Id for each PictureBoxUC control in the designer of Form1.

It is important to understand that the Id doesn't represent a PictureBoxUC user control instance (unlike your original code where you tried to store pictureBox control instances in an array).

Instead of tracking user controls, I've created a singleton class called a Model and it tracks the state info for each Id (i.e. each Machine). This is kind of a view model architecture and although it is very simple, because it only tracks a [machine] id, and whether it's been selected, the concept can be expanded to set a collection of data per machine.

Once I created the PictureBoxUC control, I dragged a couple of them onto the Form1 and set their [machine] Id's in the properties page and also wired up the SelChanged event. Because I'm passing in the Id with each SelChange event, I'm able to wire up each control to the same SelChanged event handler. This makes the code nice and tidy.

Code:

---


privatevoid  OnSelChanged( object sender, PictureBoxSelChangedEventArgs e )
{
  Model.Instance.SetState( e.Id, e.IsSelected );
}


---

You'll notice that within this handler I call Model.Instance.SetState( ). This calls the SetState method in the singleton Model class and stores the Id, and selection state into a dictionary.

In order to prove that the states are getting stored properly, I've added a View button which displays a MessageBox with the Id's and their selected states. This just loops through a States array that I've expose from the Dictionary in the Model. Normally I wouldn't expose a dictionary like this, but this is sample code.

Code:

---


privatevoid  _viewBtn_Click( object sender, EventArgs e )
{
  var sb = newStringBuilder( );

  foreach ( var kvp inModel.Instance.States )
  {
    sb.AppendFormat( "Id: {0}\tIsSelected: {1}\n", kvp.Key, kvp.Value );
  }
  MessageBox.Show( sb.ToString( ) );
}


---

Thank you very much Arjay. I really appreciate the time you have given me to help me. :)

I will implement this method for sure, thank you again!