patterncsharpMinor
User can choose to add one of two node types to a treeView, both of which have almost identical settings
Viewed 0 times
cansettingsalmostuserchoosenodeidenticalbothonetwo
Problem
There is a context menu that comes up when a user right-clicks on a node in a TreeView. They are then presented with the choice to either add what is called a "RWB" or an "AQ" node to the tree. Both of these types of nodes have several settings in them, all of which are identical, save for one: AQ nodes have their 'Analog' field set to true, while RWBs have it set to false. Because of this, my event handling methods as well as my methods to add the actual node to the tree are essentially identical with the exception of changing the analog value to true within the
The problem is I can't figure out a smart way to accomplish what I need to without duplicating both the event and adding methods twice. Does anyone have any tips on how I can make this cleaner?
```
///
/// Updates the rcontext menu based on the type of item
/// selected in the relay word view.
///
private void UpdateRelayWordPagesAndContextMenu()
{
if (NavigationControlRelayWord.SelectedNode != null)
{
NavigationControlRelayWord.ContextMenu = new ContextMenu();
if (NavigationControlRelayWord.SelectedNode.Parent == null)
{
// Root node
NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.AddCategory, AddRWBAQCategoryEvent);
NavigationControlRelayWord.ContextMenu.MenuItems.Add("-");
NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.PasteRWBAQNode, PasteRWBAQEvent);
RelayWordNodeValue clipboardNodeValue = ClipboardUtilities.GetRWBAQClipboardNode();
if (string.IsNullOrWhiteSpace(clipboardNodeValue.Name) ||
!RelayWordNodeValue.CategoryType.Equals(clipboardNodeValue.NodeType))
{
NavigationControlRelayWord.ContextMenu.MenuItems[NavigationControlRelayWord.ContextMenu.MenuItems.Count - 1].Enabled = false;
}
}
else if (NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicCategory)
{
// Category node
NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resource
AddAQ() method.The problem is I can't figure out a smart way to accomplish what I need to without duplicating both the event and adding methods twice. Does anyone have any tips on how I can make this cleaner?
```
///
/// Updates the rcontext menu based on the type of item
/// selected in the relay word view.
///
private void UpdateRelayWordPagesAndContextMenu()
{
if (NavigationControlRelayWord.SelectedNode != null)
{
NavigationControlRelayWord.ContextMenu = new ContextMenu();
if (NavigationControlRelayWord.SelectedNode.Parent == null)
{
// Root node
NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.AddCategory, AddRWBAQCategoryEvent);
NavigationControlRelayWord.ContextMenu.MenuItems.Add("-");
NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.PasteRWBAQNode, PasteRWBAQEvent);
RelayWordNodeValue clipboardNodeValue = ClipboardUtilities.GetRWBAQClipboardNode();
if (string.IsNullOrWhiteSpace(clipboardNodeValue.Name) ||
!RelayWordNodeValue.CategoryType.Equals(clipboardNodeValue.NodeType))
{
NavigationControlRelayWord.ContextMenu.MenuItems[NavigationControlRelayWord.ContextMenu.MenuItems.Count - 1].Enabled = false;
}
}
else if (NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicCategory)
{
// Category node
NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resource
Solution
Early returns
Early returns are your friend:
VS:
Now you have one less level of indentation to keep straight.
You can also user early returns here (
I would write this as:
Methods
Use more methods! For example,
These can be extracted into their own methods, along with a lot of other code.
Early returns are your friend:
private void UpdateRelayWordPagesAndContextMenu()
{
if (NavigationControlRelayWord.SelectedNode != null)
{
// Really long block here....
}
}VS:
private void UpdateRelayWordPagesAndContextMenu()
{
if (NavigationControlRelayWord.SelectedNode == null)
{
return;
}
// Really long block here
}Now you have one less level of indentation to keep straight.
You can also user early returns here (
AddAQEvent(object sender, EventArgs e)):if (inputForm.ShowDialog() == DialogResult.OK)
{
var name = inputForm.ShortName;
var description = inputForm.DescriptiveName;
if (name != string.Empty)
{
if (Device.DeviceWord.FindItemByName(name) == null)
{
AddAnalogQuantityToTreeView(name, description);
}
else
{
MessageBox.Show(Resources.ElementExists, Resources.DeviceBuilder);
}
}
else
{
MessageBox.Show(Resources.BlankAQError, Resources.DeviceBuilder);
}
}I would write this as:
if (inputForm.ShowDialog() == DialogResult.OK)
{
var name = inputForm.ShortName;
var description = inputForm.DescriptiveName;
if (name == string.Empty)
{
MessageBox.Show(Resources.BlankAQError, Resources.DeviceBuilder);
return;
}
if (Device.DeviceWord.FindItemByName(name) == null)
{
AddAnalogQuantityToTreeView(name, description);
}
else
{
MessageBox.Show(Resources.ElementExists, Resources.DeviceBuilder);
}
}Methods
Use more methods! For example,
UpdateRelayWordPagesAndContextMenu():if (NavigationControlRelayWord.SelectedNode.Parent == null)
{
// block here
}
else if (NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicCategory)
{
// block here
}
else if (NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicElement)
{
// block here
}These can be extracted into their own methods, along with a lot of other code.
Code Snippets
private void UpdateRelayWordPagesAndContextMenu()
{
if (NavigationControlRelayWord.SelectedNode != null)
{
// Really long block here....
}
}private void UpdateRelayWordPagesAndContextMenu()
{
if (NavigationControlRelayWord.SelectedNode == null)
{
return;
}
// Really long block here
}if (inputForm.ShowDialog() == DialogResult.OK)
{
var name = inputForm.ShortName;
var description = inputForm.DescriptiveName;
if (name != string.Empty)
{
if (Device.DeviceWord.FindItemByName(name) == null)
{
AddAnalogQuantityToTreeView(name, description);
}
else
{
MessageBox.Show(Resources.ElementExists, Resources.DeviceBuilder);
}
}
else
{
MessageBox.Show(Resources.BlankAQError, Resources.DeviceBuilder);
}
}if (inputForm.ShowDialog() == DialogResult.OK)
{
var name = inputForm.ShortName;
var description = inputForm.DescriptiveName;
if (name == string.Empty)
{
MessageBox.Show(Resources.BlankAQError, Resources.DeviceBuilder);
return;
}
if (Device.DeviceWord.FindItemByName(name) == null)
{
AddAnalogQuantityToTreeView(name, description);
}
else
{
MessageBox.Show(Resources.ElementExists, Resources.DeviceBuilder);
}
}if (NavigationControlRelayWord.SelectedNode.Parent == null)
{
// block here
}
else if (NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicCategory)
{
// block here
}
else if (NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicElement)
{
// block here
}Context
StackExchange Code Review Q#94707, answer score: 4
Revisions (0)
No revisions yet.