16
\$\begingroup\$

Inspired by this question in chat

Is there an easy way in .NET to split a string into an array by pulling chunks of the text in. I.e. I have an 18 character string I would like in 3 6-character pieces.

I thought that ain't be that hard. I would like to get any thoughts and improvements you can think of.

public static string[] Split(this string value, int desiredLength, bool strict = false)
{

    EnsureValid(value, desiredLength, strict);

    if (value.Length == 0) { return new string[0]; }

    int numberOfItems = value.Length / desiredLength;

    int remaining = (value.Length > numberOfItems * desiredLength) ? 1 : 0;

    IList<string> splitted = new List<string>(numberOfItems + remaining);

    for (int i = 0; i < numberOfItems; i++)
    {
        splitted.Add(value.Substring(i * desiredLength, desiredLength));
    }

    if (remaining != 0)
    {
        splitted.Add(value.Substring(numberOfItems * desiredLength));
    }

    return splitted.ToArray();
}

private static void EnsureValid(string value, int desiredLength, bool strict)
{
    if (value == null) { throw new ArgumentNullException(nameof(value)); }

    if (value.Length == 0 && desiredLength != 0)
    {
        throw new ArgumentException($"The passed {nameof(value)} may not be empty if the {nameof(desiredLength)} <> 0");
    }

    if (value.Length != 0 && desiredLength < 1) { throw new ArgumentException($"The value of {nameof(desiredLength)} needs to be > 0"); }

    if (strict && (value.Length % desiredLength != 0))
    {
        throw new ArgumentException($"The passed {nameof(value)}'s length can't be split by the {nameof(desiredLength)}");
    }
}

I know the tests aren't complete and the naming could be better, like the missing of a test if the passed in value == null, but here they are for review as well

[TestMethod()]
public void SplitTest()
{
    string input = "123456";
    string[] expected = new string[] { "123", "456" };
    string[] actual = input.Split(3);

    CollectionAssert.AreEqual(expected, actual);
}

[TestMethod()]
public void SplitTestNotEqual()
{
    string input = "12345";
    string[] expected = new string[] { "123", "45" };
    string[] actual = input.Split(3);

    CollectionAssert.AreEqual(expected, actual);
}

[TestMethod()]
[ExpectedException(typeof(ArgumentException))]
public void SplitTestDesiredlengthIsZero()
{
    string input = "12345";
    string[] actual = input.Split(0);
}

[TestMethod()]
public void SplitTestDesiredlengthIsZeroWithEmptyString()
{
    string input = "";
    string[] expected = new string[0];
    string[] actual = input.Split(0);

    CollectionAssert.AreEqual(expected, actual);
}

[TestMethod()][ExpectedException(typeof(ArgumentException))]
public void SplitTestWillThrowBecauseOfStrict()
{
    string input = "12345";
    string[] actual = input.Split(3,true);
}
\$\endgroup\$
14
+100
\$\begingroup\$

Algorithm can't be improved (or at least I see no reason to do it, ignoring a minor micro-optimization about List<string> which usage, however, helps to keep code much more readable) and code is pretty clear, however:

  • You're returning string[] but consumers may not need it. List.ToArray() won't simply return internal array (because its capacity != size). It implies you will make another (relatively expansive) copy. I'd simply return IEnumerable<string>: if consumer needs an array it can call ToArray() LINQ extension method (which will/may check for underlying implementation), it he doesn't need an array then you saved an Array.Copy().
  • You're partitioning over Char but String is UTF-16 encoded then you may produce broken strings in, at least, three cases:

    1. One character is encoded with more than one code unit. Unicode code point for that character is encoded as two UTF-16 code units, each code unit may end up in two different slices (and both strings will be invalid).
    2. One character is composed by more than one code point. You're dealing with a character made by two separate Unicode code points (for example Han character 𠀑).
    3. One character has combining characters or modifiers. This is more common than you may think: for example Unicode combining character like U+0300 COMBINING GRAVE ACCENT used to build à and Unicode modifiers such as U+02BC MODIFIER LETTER APOSTROPHE.
  • Definition of character for a programming language and for a human being are pretty different, for example in Czech Slovak (thanks svick to correct me about this) is a single character however it's made by 2/3 Unicode code points which are in this case also 2/3 UTF-16 code units then "dž".Length > 1. More about this and other cultural issues on this Stack Overflow post.
  • Ligatures exist. Assuming one ligature is one code point (and also assuming it's encoded as one code unit) then you will treat it as a single glyph however it represents two characters. What to do in this case? In general definition of character may be pretty vague because it has a different meaning according to discipline where this word is used. You can't (probably) handle everything correctly but you should set some constraints and document code behavior.

To address all these issues (also third point if current culture is correctly set but not last one) I'd use:

string[] characters = StringInfo.GetTextElementEnumerator(value).ToArray();

And I'd perform subsequent processing over characters array instead of value. Note that this may lead to strings made of different length but with the same number of characters. For example (when performance are not an issue):

public static IEnumerable<string> Split(this string value, int desiredLength)
{
    var characters = StringInfo.GetTextElementEnumerator(value);
    while (characters.MoveNext())
        yield return String.Concat(Take(characters, desiredLength));
}

private static IEnumerable<string> Take(TextElementEnumerator enumerator, int count)
{
    for (int i = 0; i < count; ++i)
    {
        yield return (string)enumerator.Current;

        if (!enumerator.MoveNext())
            yield break;
    }
}

To do not reinvent the wheel you may just create your own AsEnumerable() extension method to walk through an existing enumerator:

public static IEnumerable<T> AsEnumerable<T>(this IEnumerator enumerator)
{
    while (enumerator.MoveNext())
        yield return (T)enumerator.Current;
}

Your code will be then simplified to:

public static IEnumerable<string> Split(this string value, int desiredLength)
{
    var characters = StringInfo.GetTextElementEnumerator(value);
    do
    {
        yield return String.Concat(characters.AsEnumerable<string>().Take(desiredLength));
    } while (characters.MoveNext());
}

If you don't need string[] return type you may live with IEnumerable<string> and use LINQ ToArray() when/if required (code is slightly simplified for demonstrative purposes, in real world you will need some error checking).

If you're processing US ASCII text and you won't need to partition horse d'œvre then you can simply ignore these issues.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot. The unicode part is quite enlightened. Do you maybe have some link(s) to read more about these kind of problems ? \$\endgroup\$ – Heslacher Nov 26 '15 at 16:24
  • 1
    \$\begingroup\$ I wrote little bit about this on a Stack Overflow post: stackoverflow.com/a/27229590/1207195 but it's far to be complete (and should be improved a lot). It's a community wiki then feel free to add/edit/fix whatever you want. \$\endgroup\$ – Adriano Repetti Nov 26 '15 at 16:30
  • \$\begingroup\$ Some clarifications from a Czech: is not a character in Czech (though it seems to be a character in Slovak). And it's written using two code points (d + ž), unless you use the aforementioned combining characters. \$\endgroup\$ – svick Nov 26 '15 at 19:06
  • 2
    \$\begingroup\$ +1 - it's always nice to see someone else commenting about unicode. For the record, I always use the canonical form of e acute as my example: var eAcute = "e\u0301"; // é because it's a letter that is used in english, e.g. café, so more readily grokkable (for english speakers). \$\endgroup\$ – RobH Nov 27 '15 at 8:47
  • 1
    \$\begingroup\$ @RobH I agree, I don't do it just because I got it on my keyboard but if those duplicates were dropped in favor of combining characters only then our life would be easier. I'm not sure (Heslacher may help us about this) but with German keyboards they may have both forms. \$\endgroup\$ – Adriano Repetti Nov 27 '15 at 8:55
5
\$\begingroup\$

There are only two things I can suggest:

  1. You don't need to create a List<string> first and then copy it to an array via ToArray() extension method. You know an exact number of elements in the output, so it should be just an array string[].
  2. Since there are several methods named Split in the String class, it may be a little confusing. Consider to rename your method.
\$\endgroup\$
4
\$\begingroup\$

Although @AdrianoRepetti's answer is addressing some very important points, the suggested code

string[] characters = StringInfo.GetTextElementEnumerator(value).ToArray();   

just didn't happen to exists. Nevertheless his answer gave me a headstart for an answer to my question.

So I digged a little bit into the StringInfo class and discovered that there lives a very handy method, namely the SubstringByTextElements() method which seems to serve my purpose well.

In addition the LengthInTextElements property needs to replace the calls to the string.Length property to make the whole thing work.


By reading @Dmitri's answer I realized that he is right about

You know an exact number of elements in the output, so it should be just an array string[].

Unfortunately I can't agree about the naming, because just because the string class already has a Split() method I want to provide this as an overloaded one.


So after applying the mentioned points the methods will look like so

public static string[] Split(this string value, int desiredLength, bool strict = false)
{

    EnsureValid(value, desiredLength, strict);

    var stringInfo = new StringInfo(value);

    int currentLength = stringInfo.LengthInTextElements;
    if (currentLength == 0) { return new string[0]; }

    int numberOfItems = currentLength / desiredLength;

    int remaining = (currentLength > numberOfItems * desiredLength) ? 1 : 0;

    var chunks = new string[numberOfItems + remaining];

    for (var i = 0; i < numberOfItems; i++)
    {
        chunks[i] = stringInfo.SubstringByTextElements(i * desiredLength, desiredLength);
    }

    if (remaining != 0)
    {
        chunks[numberOfItems] = stringInfo.SubstringByTextElements(numberOfItems * desiredLength);
    }

    return chunks;
}
private static void EnsureValid(string value, int desiredLength, bool strict)
{
    if (value == null) { throw new ArgumentNullException(nameof(value)); }

    if (value.Length == 0 && desiredLength != 0)
    {
        throw new ArgumentException($"The passed {nameof(value)} may not be empty if the {nameof(desiredLength)} <> 0");
    }

    var info = new StringInfo(value);
    int valueLength = info.LengthInTextElements;

    if (valueLength != 0 && desiredLength < 1) { throw new ArgumentException($"The value of {nameof(desiredLength)} needs to be > 0"); }

    if (strict && (valueLength % desiredLength != 0))
    {
        throw new ArgumentException($"The passed {nameof(value)}'s length can't be split by the {nameof(desiredLength)}");
    }
}

which aren't only passing the former tests, but also this new tests

[TestMethod()]
public void SplitTestUnicodeVariant()
{
    string input = "𠀑𠀑𠀑a𠀑𠀑𠀑";
    string[] expected = new string[] { "𠀑𠀑", "𠀑a", "𠀑𠀑", "𠀑" };
    string[] actual = input.Split(2);
    CollectionAssert.AreEqual(expected, actual);
}
[TestMethod()]
public void SplitTestUnicodeVariant1()
{
    string input = "dž𠀑𠀑a𠀑é𠀑";
    string[] expected = new string[] { "dž𠀑", "𠀑a𠀑", "é𠀑" };
    string[] actual = input.Split(3);
    CollectionAssert.AreEqual(expected, actual);
}
[TestMethod()]
public void SplitTestUnicodeVariant2()
{
    string input = "éée\u0301éé";
    string[] expected = new string[] { "éé", "e\u0301é", "é" };
    string[] actual = input.Split(2);
    CollectionAssert.AreEqual(expected, actual);
}
\$\endgroup\$
  • \$\begingroup\$ @AdrianoRepetti the GetTextElementEnumerator() method exists, but there is no way to call ToArray() on the result because the result is a TextElementEnumerator on which one would need to call GetTextElement() prior to calling ToArray(). \$\endgroup\$ – Heslacher Nov 30 '15 at 5:43
3
\$\begingroup\$

There are a few things that popped in my mind seeing this:

Generalization

The method can be used to split any enumerable/collection/array, but this is just a minor point.

EnsureValid

IMO this method does a little too much. It: checks if the parameters are valid (desiredLength != 0, which I think should also be desiredLength > 0 given the context) and also takes action to prevent the application from going on (throws exceptions, in this case). I'd change it into something like the following:

private static bool IsValid(string value, int desiredLength, bool strict)
{
    return value != null
        && !(value.Length == 0 && desiredLength != 0)
        && !(value.Length != 0 && desiredLength < 1)
        && !(strict && (value.Length % desiredLength != 0));
}

and let the management of special cases to the caller. In this way, the first line of the Split method becomes something like the following:

public static string[] Split(this string value, int desiredLength, bool strict = false)
{
    if(!IsValid(value, desiredLength, strict))
    {
        // manage the non valid case here
    }

    // other logic
}

Split method

In the current case, for strings I mean, the method seems good. In case you consider changing it for the general case, I'd suggest the following logic:

public static T[] Split<T>(this T[] value, int desiredLength, bool strict = false)
{
    if(!IsValid(value, desiredLength, strict))
    {
        // manage invalid case
    }

    var numberOfItems = (int)Math.Ceiling((double)value.Length / (double)desiredLength);
    var result = new T[numberOfItems];
    var chunkItems = new List<T>();

    for(int i = 0; i < numberOfItems; i++)
    {
        int chunkFirstIndex = i * desiredLength;

        for(int j = 0, currentIndex = chunkFirstIndex; 
            j < desiredLength && currentIndex < value.Length; 
            j++, currentIndex++)
        {
            chunkItems.Add(value[currentIndex]);
        }

        result[i] = chunkItems.ToArray();
        chunkItems.Clear();
    }

    return result;
}

Other than these things, everything else looks fine.

P.S: The code I wrote is not tested, it's there just to better explain the idea.

\$\endgroup\$
  • 3
    \$\begingroup\$ I don't agree with the IsValid() method. The EnsureValid() method does exactly what it should by ensuring that the passed arguments are valid. If they aren't valid the program flow is interupted by an exception. It can be seen like some Assert.() method. \$\endgroup\$ – Heslacher Nov 27 '15 at 11:10
  • \$\begingroup\$ @Heslacher I see your point. I saw it more like an Assert.IsTrue(IsValid()) \$\endgroup\$ – Gentian Kasa Nov 28 '15 at 9:20

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.