Header iterator memory optimization in HeaderGroup

[ok2c]

@arturobernalg Please double-check / review

[arturobernalg]

@ok2c
The new BasicListHeaderIterator constructor does not initialize lastIndex, so it defaults to 0. Calling remove() before next() then removes the first header instead of throwing IllegalStateException. I confirmed it with a regression test. Initializing lastIndex to -1 fixes it.

Otherwise, the change looks good to me.

Test that prove.

@Test
void testIteratorByNameRemoveBeforeNext() {
    final HeaderGroup headerGroup = new HeaderGroup();
    final Header headerA = new BasicHeader("a", "a-one");
    final Header headerB = new BasicHeader("b", "b-one");
    headerGroup.setHeaders(headerA, headerB);

    final Iterator<Header> iterator = headerGroup.headerIterator("b");

    Assertions.assertThrows(IllegalStateException.class, iterator::remove);
    Assertions.assertArrayEquals(
            new Header[] { headerA, headerB },
            headerGroup.getHeaders());
}

[ok2c]

@arturobernalg Good catch! Thank you! Please do another pass. I have also made a few small improvements and optimizations and have added test coverage. Those tests can also serve as an example of how to parse messages efficiently

[arturobernalg]

@ok2c The previous implementation inherited argument validation from parseElementList(). With the direct parsing loop those checks are lost.

Could we preserve the existing contract here?

Args.notNull(src, "Source");
Args.notNull(cursor, "Cursor");
Args.notNull(consumer, "Consumer");

Otherwise look good

[ok2c]

@ok2c The previous implementation inherited argument validation from parseElementList(). With the direct parsing loop those checks are lost.

Could we preserve the existing contract here?

@arturobernalg I can do that, but would that be really useful? This is a package private constructor not exposed through public APIs and not callable outside of the same package. What good would those checks do?

[arturobernalg]

@ok2c The previous implementation inherited argument validation from parseElementList(). With the direct parsing loop those checks are lost.
Could we preserve the existing contract here?

@arturobernalg I can do that, but would that be really useful? This is a package private constructor not exposed through public APIs and not callable outside of the same package. What good would those checks do?

@ok2c Fair enough

[arturobernalg]

The previous implementation inherited argument validation from parseElementList(). With the direct parsing loop those checks are lost.

Could we preserve the existing contract here?

Args.notNull(src, "Source");
Args.notNull(cursor, "Cursor");
Args.notNull(consumer, "Consumer");

Otherwise look good