patterncsharpMinor
Creating submenus with buttons
Viewed 0 times
creatingbuttonswithsubmenus
Problem
I have a menu, where a user selects one out of 4, and another 2 or 4 options will appear. I used big buttons; a picturebox + label to create the buttons.
The 4 main buttons are fixed, but the 4 other buttons depend on the first choice. Text and image will change. I programmed this, but I kinda feel this must be possible more easily.
I have 4 structs which contain the image path and label text. like this: (I'll only show with 2, because the rest won't add anything, it is just the same.)
Then I have the function where I make the buttons: (it is called when press on the main buttons)
```
private void CreateSubButtons(string sender)
{
pictureBoxSubBtn3.Image = null;
pictureBoxSubBtn4.Image = null;
SubBtn3Text.Text = "";
SubBtn4Text.Text = "";
panelSubBtn3.BorderStyle = BorderStyle.None;
panelSubBtn4.BorderStyle = BorderStyle.None;
switch (sender)
{
case _mainButton1:
SubBtn1Text.Text = ConfigDPSubmenu._SubMenuBtn1Text;
SubBtn2Text.Text = ConfigDPSubmenu._SubMenuBtn2Text;
pictureBoxSubBtn1.Image = Image.FromFile(ConfigDPSubmenu._SubMenuBtn1Img);
pictureBoxSubBtn2.Image = Image.FromFile(ConfigDPSubmenu._SubMenuBtn2Img);
panelSubBtn1.BorderStyle = BorderStyle.FixedSingle;
The 4 main buttons are fixed, but the 4 other buttons depend on the first choice. Text and image will change. I programmed this, but I kinda feel this must be possible more easily.
I have 4 structs which contain the image path and label text. like this: (I'll only show with 2, because the rest won't add anything, it is just the same.)
private const string _mainButton1 = "ConfigDP";
private const string _mainButton2 = "ConfigADP";
struct ConfigDPSubmenu
{
public const string _SubMenuBtn1Text = "text submenu 1 button 1";
public const string _SubMenuBtn2Text = "text submenu 1 button 2";
public const string _SubMenuBtn1Img = "DevPortal.png";
public const string _SubMenuBtn2Img = "CustomDevPortal.png";
}
struct ConfigADPSubmenu
{
public const string _SubMenuBtn1Text = "text submenu 2 button 1";
public const string _SubMenuBtn2Text = "text submenu 2 button 2";
public const string _SubMenuBtn1Img = "ADP.png";
public const string _SubMenuBtn2Img = "CustomADP.png";
}Then I have the function where I make the buttons: (it is called when press on the main buttons)
```
private void CreateSubButtons(string sender)
{
pictureBoxSubBtn3.Image = null;
pictureBoxSubBtn4.Image = null;
SubBtn3Text.Text = "";
SubBtn4Text.Text = "";
panelSubBtn3.BorderStyle = BorderStyle.None;
panelSubBtn4.BorderStyle = BorderStyle.None;
switch (sender)
{
case _mainButton1:
SubBtn1Text.Text = ConfigDPSubmenu._SubMenuBtn1Text;
SubBtn2Text.Text = ConfigDPSubmenu._SubMenuBtn2Text;
pictureBoxSubBtn1.Image = Image.FromFile(ConfigDPSubmenu._SubMenuBtn1Img);
pictureBoxSubBtn2.Image = Image.FromFile(ConfigDPSubmenu._SubMenuBtn2Img);
panelSubBtn1.BorderStyle = BorderStyle.FixedSingle;
Solution
A couple of points that may help you.
-
When you see yourself copy and pasting or writing duplicated code, consider refactoring. I could only see one difference in your code in the case statements. You could move all that to a method and pass in the necessary struct. Even further, once you have done this you will see further code duplication you could probably further refactor into seperate methods.
-
I'm not sure about having exposed fields on the struct start with _. You might want to read Jon Skeets Answer on stack overflow.
-
I would probably consider removing your struct and using a single class. Then I would create 4 instances of that class with the values required. I might even consider populating the text for those from a resource file.
-
I'm not really sure on this, but in old versions (Vb 6....) you could create controls which were arrays of each other. So instead of panelSubBtn1, panelSubBtn2 etc you would just have panelSubBtn and reference it by panelSubBtn[0] etc If this was possible you could seriously simplify your code. I'll let you figure that out though...
These are a few quick things which may or may not help.
-
When you see yourself copy and pasting or writing duplicated code, consider refactoring. I could only see one difference in your code in the case statements. You could move all that to a method and pass in the necessary struct. Even further, once you have done this you will see further code duplication you could probably further refactor into seperate methods.
-
I'm not sure about having exposed fields on the struct start with _. You might want to read Jon Skeets Answer on stack overflow.
-
I would probably consider removing your struct and using a single class. Then I would create 4 instances of that class with the values required. I might even consider populating the text for those from a resource file.
-
I'm not really sure on this, but in old versions (Vb 6....) you could create controls which were arrays of each other. So instead of panelSubBtn1, panelSubBtn2 etc you would just have panelSubBtn and reference it by panelSubBtn[0] etc If this was possible you could seriously simplify your code. I'll let you figure that out though...
These are a few quick things which may or may not help.
Context
StackExchange Code Review Q#17834, answer score: 7
Revisions (0)
No revisions yet.