7
\$\begingroup\$

Problem

A single line should be separated by Environment.NewLine so that each row has a length of columns (expect for the last one that may be shorter).

Below is my solution:

    public static string Wrap(this string singleLineString, int columns)
    {
        if (singleLineString == null)
            throw new ArgumentNullException("singleLineString");
        if (columns < 1)
            throw new ArgumentException("'columns' must be greater than 0.");

        var rows = Math.Ceiling((double)singleLineString.Length/columns);
        if (rows < 2) return singleLineString;

        var sb = new StringBuilder();

        for (int i = 0; i < rows; i++)
        {
            if (i > 0) sb.Append(Environment.NewLine);
            var index = i*columns;
            var length = Math.Min(columns, singleLineString.Length - index);
            var line = singleLineString.Substring(index, length);
            sb.Append(line);
        }

        return sb.ToString();
    }

Even if the problem is really simple, there are lots of thinkable approaches to solve it. Is there simpler / more elegant solution than the one above?

\$\endgroup\$
6
\$\begingroup\$

I think you shouldn't build a new string there because it's a new responsibility. The method should only split the text and return its parts with the specified length.

The user should decide what to do with the results next. Maybe he does not want to use the Environment.NewLine but some other new-line?

I prefer it to be a split-only method:

public static IEnumerable<string> Split(this string text, int partLength)
{
    if (text == null) { throw new ArgumentNullException("singleLineString"); }

    if (partLength < 1) { throw new ArgumentException("'columns' must be greater than 0."); }

    var partCount = Math.Ceiling((double)text.Length / partLength);
    if (partCount < 2)
    {
        yield return text;
    }

    for (int i = 0; i < partCount; i++)
    {
        var index = i * partLength;
        var lengthLeft = Math.Min(partLength, text.Length - index);
        var line = text.Substring(index, lengthLeft);
        yield return line;
    }
}

I need to complain about the missing {} but I guess you probably know that ;-)

The names of the variables could also be a little bit clearer.

Technically you don't need this if:

    if (partCount < 2)
    {
        yield return text;
    }

The result will be the same with out it because the for would execute only once .

| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ A possible alternative implementation: stackoverflow.com/a/1450797/592182 \$\endgroup\$ – forsvarir Sep 16 '16 at 7:54
  • \$\begingroup\$ @forsvarir I was thinking about something like this too but I was too lazy to actually write it :-] \$\endgroup\$ – t3chb0t Sep 16 '16 at 8:06
  • \$\begingroup\$ @t3chb0t: indeed, that is more flexible - thanks! I added another method Split that contains the main logic. :) You say 'missing', I would say 'needless'.. but that is another discussion ;) \$\endgroup\$ – JanDotNet Sep 16 '16 at 8:21
  • \$\begingroup\$ @forsvarir: The code of the accepted answer didn't work, but the second code fragment of the answer below did :) - thanks. \$\endgroup\$ – JanDotNet Sep 16 '16 at 8:22
6
\$\begingroup\$

Considering the answer from t3chb0t and forsvarir's suggestion for the alternative implementation, I came up with the following solution:

    public static string Wrap(this string singleLineString, int columns)
        => string.Join(Environment.NewLine, singleLineString.Split(columns));

    public static IEnumerable<string> Split(this string str, int chunkSize)
    {
        if (str == null)
            throw new ArgumentNullException("str");
        if (chunkSize < 1)
            throw new ArgumentException("'chunkSize' must be greater than 0.");

        for (int i = 0; i < str.Length; i += chunkSize)
            yield return str.Substring(i, Math.Min(chunkSize, str.Length - i));
    }
| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

Since LINQ can do pretty much anything, except for maybe flying a spaceship... It can definitely split a line into columns:

public static string Wrap(this string singleLineString, int columns)
{
    if (singleLineString == null)
        throw new ArgumentNullException("singleLineString");
    if (columns < 1)
        throw new ArgumentException("'columns' must be greater than 0.");

    var rows = (int)Math.Ceiling((double)singleLineString.Length / columns);
    if (rows < 2) return singleLineString;

    return String.Join(
      Environment.NewLine,
      Enumerable.Range(0, rows)
          .Select(i => i*columns)
          .Select(i => singleLineString
             .Substring(i, Math.Min(columns, singleLineString.Length - i)))
    );
}

How's that for elegant?

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ You could use that method to wrap the last statement so that scrolling is not required anymore ;) \$\endgroup\$ – JanDotNet Sep 16 '16 at 9:34
  • \$\begingroup\$ Probably LINQ's query syntax is also a good option because of the nested intermediate computations!? \$\endgroup\$ – JanDotNet Sep 16 '16 at 9:43
  • 2
    \$\begingroup\$ a bit of horizontal scrolling required haha ;-) you can use these line breaks if you want \r\n, \r\n, \r\n, \r\n, \r\n, \r\n, \r\n, \r\n, \r\n - I have plenty of them ;-P \$\endgroup\$ – t3chb0t Sep 16 '16 at 9:57
  • \$\begingroup\$ Where is the fun in that?! :) On more serious note, for some special cases LINQ can be a viable option, but as you can see, for general case it is not very readable. I doubt query syntax can change that, since it will roughly translate into the same thing (except for first Select transforming into let). I would go for regular loop, as other answers suggested. \$\endgroup\$ – Nikita B Sep 16 '16 at 10:11

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