Console word wrapping

Question

I wrote a text adventure game a while back and today I looked at it to see how I did the word wrapping (to use in a new project); I'm curious if I went about it the right way. I realize I could use an array instead of a list and remove 2 dependencies from the class but wanted some other feedback as I may have missed something obvious.

    public static string GetBorderedText(string Input, int AbsoluteLength, bool splitLines = true, int Offset = -4)
    {
        string outputString = "";

        if(Input.Length > AbsoluteLength + Offset && splitLines)
        {
            List<string> words = Input.Split(' ').ToList();
            string lineConstruction = "";

            foreach (string s in words)
            {
                if (lineConstruction.Length + s.Length + 1 >= AbsoluteLength + Offset)
                {
                    outputString += "+ " + GetPaddedSubstring(lineConstruction, AbsoluteLength, Offset) + " +" + Environment.NewLine;
                    lineConstruction = "";
                    lineConstruction += s + " ";
                }
                else
                {
                    lineConstruction += s + " ";
                }
            }

            outputString += "+ " + GetPaddedSubstring(lineConstruction, AbsoluteLength, Offset) + " +";

            return outputString;
        }
        else
        {
            outputString = "+ " + GetPaddedSubstring(Input, AbsoluteLength, Offset) + " +";
            return outputString;
        }
    }

where GetPaddedSubstring is used for getting a substring padded to a certain length.

    public static string GetPaddedSubstring(string Input, int AbsoluteLength, int Offset)
    {
        int targetLength = AbsoluteLength + Offset;
        string output = "";

        if(Input.Length > targetLength)
        {
            return Input.Substring(0, targetLength);
        }
        else
        {
            output = Input.PadRight(targetLength);

            return output;
        }
    }

---

Lastly, for some examples. GetBorderedText can take in something like

"This room looks sturdy, but old. Lots of storage and containers. There is a door on the far side."

And return something like this.

+ This room looks sturdy, but old. Lots of storage and containers. There is a door     +
+ on the far side.                                                                     +

The + on the side are just to create a nice border within the Console window. (Hence the -4 default on Offset)

Answer 1

GetBorderedText(string Input, int AbsoluteLength, bool splitLines = true, int Offset = -4)

You are very inconsistent with naming parameters. Some of them are in PascalCase while only one has the correct camelCase. This makes your code look strange and unprofessional.

---

if (Input.Length > AbsoluteLength + Offset && splitLines)

Conditions like this are better stored in a variable that describes what its purpose is and split into two. Consider this:

var wrapRequired = input.Length > absoluteLength + offset;
if (wrapRequired && canWrap)

---

input.Split(' ').ToList()

You don't need to call ToList, it's already an array and can be enumerated.

---

foreach (string s in words)

If your collection is called words then name the item word and not s. It's very confusing when one looks at the code and wonders what that s is.

---

lineConstruction.Length + word.Length + 1 >= absoluteLength + offset

The same here. You should name this condition propertly and especially the magic 1.

---

Other than this

• you should use StringBuilder for concatenating strings in loops because it's not only faster but also easier to use.
• make the border as prefix & suffix parameters instead of hardcoding them (or leftBorder & rightBorder).
• use variables with stronger names and use more of them so that they document your code.

As an example of how you can apply all these suggestions here's the new code with one new method for adding the border.

public static string CreateBorderedText(string input, int maxLength, bool canWrap = true, string prefix = "+ ", string suffix = " +")
{
    const string wordSeparator = " ";

    var lines = new List<string>();
    var lineBuilder = new StringBuilder();
    var borderLength = prefix.Length + suffix.Length;
    var contentLength = maxLength - borderLength;

    var wrapRequired = input.Length > contentLength;
    if (wrapRequired && canWrap)
    {
        var words = input.Split(' ');
        foreach (var word in words)
        {
            var projectedLength = lineBuilder.Length + word.Length + wordSeparator.Length + borderLength;
            var wrap = projectedLength >= maxLength;
            if (wrap)
            {
                PadRight(lineBuilder, contentLength);
                AddBorder(lineBuilder, prefix, suffix);

                lines.Add(lineBuilder.ToString());
                lineBuilder.Clear();                
            }

            lineBuilder.Append(word).Append(wordSeparator);
        }
    }
    else
    {
        lineBuilder.Append(input);      
    }

    PadRight(lineBuilder, contentLength);
    AddBorder(lineBuilder, prefix, suffix);
    lines.Add(lineBuilder.ToString());

    return string.Join(Environment.NewLine, lines);
}

and the two helpers

private static void AddBorder(StringBuilder lineBuilder, string prefix, string suffix)
{
    lineBuilder
        .Insert(0, prefix)
        .Append(suffix);
}

private static void PadRight(StringBuilder lineBuilder, int targetLength)
{
    lineBuilder.Append(' ', targetLength - lineBuilder.Length);
}

Answer 2

first thing that I would do is to move the return statement outside the if else statement, because you are returning the outputString no matter what, this also leads to moving the following line outside of it's immediate if statement but to keep it inside of the foreach

lineConstruction += s + " ";

This DRYs out your code a little bit and looks like this

public static string GetBorderedText(string Input, int AbsoluteLength, bool splitLines = true, int Offset = -4)
{
    string outputString = "";

    if(Input.Length > AbsoluteLength + Offset && splitLines)
    {
        List<string> words = Input.Split(' ').ToList();
        string lineConstruction = "";

        foreach (string s in words)
        {
            if (lineConstruction.Length + s.Length + 1 >= AbsoluteLength + Offset)
            {
                outputString += "+ " + GetPaddedSubstring(lineConstruction, AbsoluteLength, Offset) + " +" + Environment.NewLine;
                lineConstruction = "";

            }
            lineConstruction += s + " ";
        }

        outputString += "+ " + GetPaddedSubstring(lineConstruction, AbsoluteLength, Offset) + " +";
    }
    else
    {
        outputString = "+ " + GetPaddedSubstring(Input, AbsoluteLength, Offset) + " +";
    }
    return outputString;
}

---

you should get rid of the output string variable from the GetPaddedSubstring() method, because you don't really need it. inside the if block you return the value immediately, but in the Else statement you assign the value to the variable and then return the value of the variable, skip that extra step and just return the value like this

public static string GetPaddedSubstring(string Input, int AbsoluteLength, int Offset)
{
    int targetLength = AbsoluteLength + Offset;
    if(Input.Length > targetLength)
    {
        return Input.Substring(0, targetLength);
    }
    else
    {
        return = Input.PadRight(targetLength);
    }
}

Answer 3

Here is your code cleaned up a bit (even removed 2 dependencies):

namespace Tests
{
    using System;

    class Program
    {
        private static void Main(string[] args)
        {
            Console.WriteLine(GetBorderedText("This room looks sturdy, but old. Lots of storage and containers. There is a door on the far side.", 80));
            Console.ReadKey();
        }

        private static string GetBorderedText(string input, int maxLength, bool splitLines = true, int offset = -4)
        {
            if (input.Length <= maxLength + offset && splitLines)
                return $"+ {GetPaddedSubstring(input, maxLength, offset)} +";

            var outputString = string.Empty;
            var lineConstruction = string.Empty;

            foreach (var s in input.Split(' '))
            {
                if (lineConstruction.Length + s.Length + 1 >= maxLength + offset)
                {
                    outputString += $"+ {GetPaddedSubstring(lineConstruction, maxLength, offset)} +{Environment.NewLine}";

                    lineConstruction = string.Empty;
                }

                lineConstruction += $"{s} ";
            }

            outputString += $"+ {GetPaddedSubstring(lineConstruction, maxLength, offset)} +";

            return outputString;
        }

        private static string GetPaddedSubstring(string input, int maxLength, int offset)
        {
            var targetLength = maxLength + offset;

            if (input.Length > targetLength)
                return input.Substring(0, targetLength);

            return input.PadRight(targetLength);

        }
    }
}

You could put a StringBuilder in there as mentioned earlier but if the number of lines is not many, I wouldn't bother.