Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

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

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 practices

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 practices

added 1224 characters in body
Source Link
Heslacher
  • 50.4k
  • 5
  • 81
  • 175

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 practices

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.

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 practices

added 5 characters in body
Source Link
Heslacher
  • 50.4k
  • 5
  • 81
  • 175

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;
    return;yield break;
}  

after the validation, in this way you wouldn't need to create a StringBuilder nor having the enumerator.

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;
    return;
}  

after the validation, in this way you wouldn't need to create a StringBuilder nor having the enumerator.

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.

added 37 characters in body
Source Link
Heslacher
  • 50.4k
  • 5
  • 81
  • 175
Loading
Source Link
Heslacher
  • 50.4k
  • 5
  • 81
  • 175
Loading