Nice and clean implementation. I tested this with the tests (the new ones) of my implementation and they all passed (skipping the tests with chunkSize == 0).
The code could be sligthly more readable by having some vertical space to group related code, for instance after validating the input.
You could improve this a little bit by just having this
if (chunkSize == 1)
{
yield return value;
yield break;
}
after the validation, in this way you wouldn't need to create a StringBuilder
nor having the enumerator
.
A slightly different approach could be to remove the for
loop. It makes the intent more clear (IMO) and removes the need to double check return value of enumerator.MoveNext()
.
Unfortunately this makes the code 2 lines (3 with the vertical spacing) longer
var sb = new StringBuilder(chunkSize);
var enumerator = StringInfo.GetTextElementEnumerator(value);
var counter = 0;
while (enumerator.MoveNext())
{
counter++;
sb.Append(enumerator.GetTextElement());
if (counter == chunkSize)
{
yield return sb.ToString();
sb.Length = 0;
counter = 0;
}
}
if (counter > 0)
{
yield return sb.ToString();
}
Regarding naming of unit tests, I usually (not in the posted question of mine) use the pattern
UnitOfWork_StateUnderTest_ExpectedBehavior
like shown in the accepted answer over here: unit test naming best practicesunit test naming best practices