5
\$\begingroup\$

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 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?

/// <summary>
/// Updates the rcontext menu based on the type of item 
/// selected in the relay word view.
/// </summary>
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(Resources.AddCategory, AddRWBAQCategoryEvent);
      NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.AddRWB, AddRWBEvent);
      NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.AddAQ, AddAQEvent);
      NavigationControlRelayWord.ContextMenu.MenuItems.Add("-");
      NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.Delete, DeleteRWBAQEvent);
      NavigationControlRelayWord.ContextMenu.MenuItems.Add("-");
      NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.CopyRWBAQNode, CopyRWBAQEvent);
      NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.PasteRWBAQNode, PasteRWBAQEvent);

      RelayWordNodeValue clipboardNodeValue = ClipboardUtilities.GetRWBAQClipboardNode();
      if (string.IsNullOrWhiteSpace(clipboardNodeValue.Name))
      {
        NavigationControlRelayWord.ContextMenu.MenuItems[NavigationControlRelayWord.ContextMenu.MenuItems.Count - 1].Enabled = false;
      }
    }
    else if (NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicElement)
    {
      // Element node
      NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.Delete, DeleteRWBAQEvent);
      NavigationControlRelayWord.ContextMenu.MenuItems.Add("-");
      NavigationControlRelayWord.ContextMenu.MenuItems.Add(Resources.CopyRWBAQNode, CopyRWBAQEvent);
    }
  }
}


/// <summary>
/// Adds a word bit to the tree-view
/// </summary>
/// <param name="name">name of the word bit</param>
/// <param name="description">description of the word bit</param>
private void AddWordBitToTreeView(string name, string description)
{
  var nodeValue = new RelayWordNodeValue();
  NavigationControlRelayWord.SelectedNode = AddRWB(NavigationControlRelayWord.SelectedNode,
                                                     nodeValue,
                                                     name,
                                                     description);      
}

/// <summary>
/// Adds an analog quantity to the tree-view
/// </summary>
/// <param name="name"></param>
/// <param name="description"></param>
private void AddAnalogQuantityToTreeView(string name, string description)
{
  var nodeValue = new RelayWordNodeValue();
  NavigationControlRelayWord.SelectedNode = AddAQ(NavigationControlRelayWord.SelectedNode,
                                                     nodeValue,
                                                     name,
                                                     description);
}

/// <summary>
/// Handles the AddRWB event of the right click menu.
/// </summary>
/// <param name="sender">The sender.</param>
/// <param name="e">The <see cref="System.EventArgs"/> instance containing the event data.</param>
private void AddRWBEvent(object sender, EventArgs e)
{
  if (NavigationControlRelayWord.SelectedNode != null &&
    NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicCategory &&
    Device != null &&
    Device.DeviceWord != null)
  {
    using (var inputForm = new InputForm
                          {
                            FormTitle = Resources.AddRWB,
                            ShortNameLabel = Resources.InternalNameLabel,
                            DescriptiveNameLabel = Resources.DescriptionLabel,
                            ShowDescription = true,
                            UpperCaseShortName = true,
                            AllowSpaceInShortName = false
                          })
    {

      if (inputForm.ShowDialog() == DialogResult.OK)
      {
        var name = inputForm.ShortName;
        var description = inputForm.DescriptiveName;

        if (name != string.Empty)
        {
          if (Device.DeviceWord.FindItemByName(name) == null)
          {
            AddWordBitToTreeView(name, description);
          }
          else
          {
            MessageBox.Show(Resources.ElementExists, Resources.DeviceBuilder);
          }
        }
        else
        {
          MessageBox.Show(Resources.BlankRWBError, Resources.DeviceBuilder);
        }
      }
    }
  }
}

/// <summary>
/// Adds a relay word bit.
/// </summary>
/// <param name="parentNode">The parent node.</param>
/// <param name="nodeValue">The node value.</param>
/// <param name="name">The relay word name.</param>
/// <param name="description">The relay word description</param>
/// <returns>The tree node created.</returns>
private TreeNode AddRWB(TreeNode parentNode, 
                          RelayWordNodeValue nodeValue, 
                          string name, 
                          string description)
{
  const string logicKindProperty = "LogicKind";
  const string unknownType = "lkUnknown";
  const string relayWordType = "lkRelayWord";

  TreeNode newNode = null;

  if (parentNode != null &&
    parentNode.Tag is IDeviceLogicCategory &&
    Device != null &&
    Device.DeviceWord != null &&
    nodeValue != null)
  {
    var parentCategory = parentNode.Tag as IDeviceLogicCategory;
    var childElement = Device.DeviceWord.CreateNewLogicElement(name);

    if (childElement != null)
    {
      childElement.PropertiesCommaTextW = nodeValue.Properties;

      // SCR00146991 Description not shown when creating RWBAQ
      // 
      // We now let the user specify the description on create/paste and use
      // the value.  Must be updated here, after nodeValue props are read.
      if (!string.IsNullOrEmpty(description))
      {
        childElement.Description = description;
      }

      // By default, make the elements readable and enabled
      childElement.Readable = true;
      childElement.Enabled = true;

      // Make it to be of type lkRelayWord by default
      var commaSeparatedProperties = childElement.PropertiesCommaTextW;
      var properties = commaSeparatedProperties.Split(',');

      var newCommaSeparatedProperties = string.Empty;
      foreach (var property in properties)
      {
        var newProperty = property;

        if (newProperty.StartsWith(logicKindProperty))
        {
          newProperty = newProperty.Replace(unknownType, relayWordType);
        }

        newCommaSeparatedProperties = newCommaSeparatedProperties + "," + newProperty;
      }

      childElement.PropertiesCommaTextW = newCommaSeparatedProperties.Trim(',');

      parentCategory.Elements.Add(childElement);

      newNode = parentNode.Nodes.Add(name);

      newNode.Tag = childElement;
      newNode.ImageIndex = Utilities.RelayWordElementImage;
      newNode.SelectedImageIndex = Utilities.RelayWordElementImage;

      NavigationControlRelayWord.Sort();
    }
  }

  return newNode;
}

/// <summary>
/// Handles the AddAQ event of the right click menu.
/// </summary>
/// <param name="sender"></param>
/// <param name="e"></param>
private void AddAQEvent(object sender, EventArgs e)
{
  if (NavigationControlRelayWord.SelectedNode != null &&
    NavigationControlRelayWord.SelectedNode.Tag is IDeviceLogicCategory &&
    Device != null &&
    Device.DeviceWord != null)
  {
    using (var inputForm = new InputForm
    {
      FormTitle = Resources.AddAQ,
      ShortNameLabel = Resources.InternalNameLabel,
      DescriptiveNameLabel = Resources.DescriptionLabel,
      ShowDescription = true,
      UpperCaseShortName = true,
      AllowSpaceInShortName = false
    })
    {

      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);
        }
      }
    }
  }
}

/// <summary>
/// Adds an Analog Quantity
/// </summary>
/// <param name="parentNode"></param>
/// <param name="nodeValue"></param>
/// <param name="name"></param>
/// <param name="description"></param>
/// <returns></returns>
private TreeNode AddAQ(TreeNode parentNode,
                          RelayWordNodeValue nodeValue,
                          string name,
                          string description)
{
  const string logicKindProperty = "LogicKind";
  const string unknownType = "lkUnknown";
  const string relayWordType = "lkRelayWord";

  TreeNode newNode = null;

  if (parentNode != null &&
    parentNode.Tag is IDeviceLogicCategory &&
    Device != null &&
    Device.DeviceWord != null &&
    nodeValue != null)
  {
    var parentCategory = parentNode.Tag as IDeviceLogicCategory;
    var childElement = Device.DeviceWord.CreateNewLogicElement(name);

    if (childElement != null)
    {
      childElement.PropertiesCommaTextW = nodeValue.Properties;

      // SCR00146991 Description not shown when creating RWBAQ
      // 
      // We now let the user specify the description on create/paste and use
      // the value.  Must be updated here, after nodeValue props are read.
      if (!string.IsNullOrEmpty(description))
      {
        childElement.Description = description;
      }

      // By default, make the elements readable and enabled
      childElement.Readable = true;
      childElement.Enabled = true;

      // By default, make the element Analog
      childElement.Analog = true;

      // Make it to be of type lkRelayWord by default
      var commaSeparatedProperties = childElement.PropertiesCommaTextW;
      var properties = commaSeparatedProperties.Split(',');

      var newCommaSeparatedProperties = string.Empty;
      foreach (var property in properties)
      {
        var newProperty = property;

        if (newProperty.StartsWith(logicKindProperty))
        {
          newProperty = newProperty.Replace(unknownType, relayWordType);
        }

        newCommaSeparatedProperties = newCommaSeparatedProperties + "," + newProperty;
      }

      childElement.PropertiesCommaTextW = newCommaSeparatedProperties.Trim(',');

      parentCategory.Elements.Add(childElement);

      newNode = parentNode.Nodes.Add(name);

      newNode.Tag = childElement;
      newNode.ImageIndex = Utilities.RelayWordElementImage;
      newNode.SelectedImageIndex = Utilities.RelayWordElementImage;

      NavigationControlRelayWord.Sort();
    }
  }

  return newNode;
}
\$\endgroup\$
4
\$\begingroup\$

Early returns

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.