1

I currently have a function that creates a stringbuilder variable using an object.

The issue that I am having is the function is now extremely long and I am beginning to realize that I can split it into multiple functions easily for readability and possibly maintainability. Would there be any reason not to split them into separate functions? Will there be a performance boost or the opposite?

Here is a smaller version of the current stringbuilder function I am using (because it is much larger around 300+ lines of code). As you can see I would be able to split into functions based on each line input:

private static StringBuilder GetObjectData(Object obj)
{
    StringBuilder sb = new StringBuilder();

    sb.AppendLine(("TempClass;" +
        obj.TempClass.TempValue +
        obj.TempClass.TempValue1 +
        obj.TempClass.TempValue2 +
        obj.TempClass.TempValue3 +
        obj.TempClass.TempValue4 +
        obj.TempClass.TempValue5 +
        obj.TempClass.TempValue6 +
        obj.TempClass.TempValue7 +
        obj.TempClass.TempValue8));

    sb.AppendLine(("TempClass2; +
        obj.TempClass2.TempValue));

    sb.AppendLine(("TempClass3;" +
        obj.TempClass3.TempValue));

    if (obj.TempClass3.TempValue != null && obj.TempClass3.TempValue1 != null)
    {
        sb.AppendLine(("TempClass3;" +
            obj.TempClass3.TempValue +
            obj.TempClass3.TempValue1));
    }

    sb.AppendLine(("TempClass4;" +
        obj.TempClass4.TempValue));

    foreach (string element in obj.TempClass5.TempValue)
    {
        sb.AppendLine(("TempClass5;" + element));
    }
    return sb;
}

Any input is much appreciated!

Erik Philips
  • 53,428
  • 11
  • 128
  • 150
Baraa
  • 687
  • 1
  • 9
  • 26
  • 8
    You might have better luck over at http://codereview.stackexchange.com/ – Brandon Sep 16 '16 at 14:08
  • There may be a slight (very slight) increase in time consumed because of the extra function calls. But it would be so minuscule in comparison to the processing that you're doing that it's irrelevant. Readable and maintainable should always be the first inclination. Worry about performance only when it's a problem. – hatchet - done with SOverflow Sep 16 '16 at 14:10
  • How many times are you calling this function? If it is not in millions then there is not much difference. – nobody Sep 16 '16 at 14:10
  • 9
    Using `+` everywhere defeats the purpose of using a StringBuilder. – Dennis_E Sep 16 '16 at 14:11
  • @Dennis_E - not necessarily. http://stackoverflow.com/questions/21078/most-effecient-way-to-concatenate-strings – hatchet - done with SOverflow Sep 16 '16 at 14:15
  • 1
    @Dennis_E Not at all. Using string concatenation is *preferable* when the number of strings to concat is known at compile time. `StringBuilder` is only beneficial when the number of strings to concat is *not* known at compile time. Given that he's concatting the strings of a fixed known number at compile time and using a `StringBuilder` when the number is unknown *he's doing it exactly right*. His solution is *preferable* to appending all of those strings. – Servy Sep 16 '16 at 14:43
  • @Brandon thanks that might be a better approach to answer code architecture related questions! I'll take a look! – Baraa Sep 16 '16 at 14:44
  • @Dennis_E hmm interesting, i'll look into it... and for simplicity sake I changed the code a bit, I actually needed + ";" + after each variable... but it would require a ton more appends. Reading Servy's reply as well, I may have to research before removing the + from my code as it will cause many more appends. – Baraa Sep 16 '16 at 14:49

3 Answers3

1

You could use a ToStringBuilder(StringBuilder sb) method for each of your sub classes. That would append to the StringBuilder they are passed, then you'd maintain the benefits of using a StringBuilder like so:

private static StringBuilder GetObjectData(MyObject obj)
{
    StringBuilder sb = new StringBuilder();

    obj.ToStringBuilder(ref sb);

    return sb;
}


class MyObject
{
    MySubObject Object1;
    MySubObject Object2;

    public void ToStringBuilder(ref StringBuilder sb)
    {
        if (Object1 != null)
        {
            sb.AppendLine(Object1.ToStringBuilder(ref sb));
        }

        if (Object2 != null)
        {
            sb.AppendLine(Object2.ToStringBuilder(ref sb));
        }
    }
}


class MySubObject
{
    object Field1;
    object Field2;

    public void ToStringBuilder(ref StringBuilder sb)
    {
        if (Field1 != null)
        {
            sb.AppendLine(Field1.ToString());
        }

        if (Field2 != null)
        {
            sb.AppendLine(Field2.ToString());
        }
    }
}
marksfrancis
  • 1,722
  • 1
  • 13
  • 14
  • This is an approach, but I actually have over 20 Sub classes. And there will be different file formats that will use different stringbuilders and I am using this object and sub objects as the neutral formats to read into and write out of all conversions. So not sure if this would be the best solution. – Baraa Sep 16 '16 at 14:52
  • @Baraa If some of your classes use .ToString(), you could use that for them instead, and append their result onto the StringBuilder? – marksfrancis Sep 16 '16 at 14:55
  • Even if I am going a different route, technically your solution is an answer to the question I posed! Thanks! – Baraa Sep 16 '16 at 16:23
0

If a function is helping maintainability and readability, you should definitely split up this code.


If you know you're going to append all parameters/fields in your class, you could shorten the code to (technically) one line:

myclass.GetType()
    .GetProperties()          //or .GetFields()
    .OrderBy(x => x.Name)
    .Where(x => ....)         //optional
    .Select(x => { sb.Append(x.GetValue(myclass).ToString()); return x; })
    .ToList();

There's a high chance that this will degrade your performance, so do some testing first, if it's good enough for you.


There's also #region somedescription + #endregion to make your code more readable.

CookedCthulhu
  • 744
  • 5
  • 14
0

If the goal is just to get a string representation of an arbitrary object you can just serialize the object to JSON:

private static StringBuilder GetObjectData(Object obj)
{
    string json = JsonConvert.SerializeObject(obj);

    return new StringBuilder(json);
}
sly
  • 300
  • 1
  • 5