Applying TrimSideSpaces() method to multiple string properties

I am looking for generic ideas and guidelines of how to improve the style of my coding and make it more readable and robust.

public class TemplatePart : ITemplatePart
{
    #region Fields

    private readonly List _options = new List();
    private readonly List _cables = new List();

    #endregion // Fields

    #region Constructor

    #endregion // Constructor

    #region Creator

    public static TemplatePart CreateTemplatePart(string partName, string basePart)
    {
        return new TemplatePart
        {
            PartName = partName,
            BasePart = basePart
        };
    }

    public static TemplatePart CreateTemplatePart(DataTable table, DataRow row, int rowIndex)
    {
        return new TemplatePart
        {
            // as you can see, TrimSideSpaces() get's applied for every single content
            // being read from the sheet for each property.
            Key = rowIndex,
            PartName = TrimSideSpaces(row.Field(Strings.TemplateSpreadSheet_Column_PartName)),
            BasePart = TrimSideSpaces(row.Field(Strings.TemplateSpreadSheet_Column_BasePart)),
            Category = TrimSideSpaces(ExtractVendor(table.TableName))

        };
    }

    private static string TrimSideSpaces(string p)
    {
        var rgx = new Regex(@"\S.*\S");
        return rgx.Match(p).Value;
    }

    #endregion // Creator

    #region Public Properties

    public int Key { get; set; }
    public string PartName { get; set; }
    public string BasePart { get; set; }
    // This captures the tab the item is in.
    public string Category { get; set; }
    public double? PriceDom { get; set; }
    public double? PriceInt { get; set; }
    public IEnumerable Options { get { return this._options; } }
    public IEnumerable Cables { get { return this._cables; } }

    #endregion // Public Properties

    #region Private Helpers
    ....
    #endregion // Private Helpers
}

I have a similar concrete class with about 20 properties and I am th

• single letter variables shouldn't be used except for loop variables

• shortening variables names removes readability of the code

• if you don't need List specific methods consider to change List _options to IList. You should if possible code against interfaces instead of implementations.

• instead of calling the TrimSideSpaces() method, which in reality does only return an empty string, you should just use the built in Trim() method of the String class which removes any leading and trailing space

Refactoring

By using the overloaded CreateTemplatePart() method

public static TemplatePart CreateTemplatePart(string partName, string basePart)
{
    return new TemplatePart
    {
        PartName = partName.Trim(),
        BasePart = basePart.Trim()
    };
}

public static TemplatePart CreateTemplatePart(DataTable table, DataRow row, int rowIndex)
{
    TemplatePart templatePart = CreateTemplatePart(row.Field(Strings.TemplateSpreadSheet_Column_PartName),
              row.Field(Strings.TemplateSpreadSheet_Column_BasePart));
    templatePart.Key = rowIndex;
    templatePart.Category = ExtractVendor(table.TableName).Trim();
    return templatePart;
}

Update based on the comments


In regards to your 4th point, TrimSideSPaces() returns the content of the property without it's leading and trailing spaces. "which in reality does only return an empty string" how?

Using the method in your code the first test fails while the second one passes.

[TestMethod()]
public void TrimSideSpacesTest_WillFail()
{
    SnippetsCodeReview target = new SnippetsCodeReview();
    string value = "     g     ";
    string expected = "g";
    string actual = target.TrimSideSpaces(value);
    Assert.AreEqual(expected, actual);
}
[TestMethod()]
public void TrimSideSpacesTest_WillPass()
{
    SnippetsCodeReview target = new SnippetsCodeReview();
    string value = "     g     ";
    string expected = String.Empty;
    string actual = target.TrimSideSpaces(value);
    Assert.AreEqual(expected, actual);
}

The suggested overloaded CreateTemplatePart() improves the code. how?

• Improvement could be the less repetitions because we use the overloaded CreateTemplatePart() method in which we call the Trim().

• Also, this "improvement" comes obviously with a lack of readability. This is partly because the Strings constants have some really long names.

An improved refactoring after renaming the overloaded methods to Create.

public static TemplatePart Create(DataTable table, DataRow row, int rowIndex)
{
    TemplatePart templatePart = Create(row);

    templatePart.Key = rowIndex;
    templatePart.Category = ExtractVendor(table.TableName).Trim();

    return templatePart;
}

private static TemplatePart Create(DataRow row)
{
    String partName = row.Field(Strings.TemplateSpreadSheet_Column_PartName);
    String basePart = row.Field(Strings.TemplateSpreadSheet_Column_BasePart);

    return Create(partName, basePart);
}

public static TemplatePart Create(string partName, string basePart)
{
    return new TemplatePart
    {
        PartName = partName.Trim(),
        BasePart = basePart.Trim()
    };
}

StackExchange Code Review Q#71994, answer score: 4