Here is one name I find can use improvement: org.apache.hc.core5.http.MessageHeaders
It's clearly a collection of things, and collections are not plural, for example: Collection, List, Set, Map. HTTP headers have an order (List) but they also have access by name (Map) but there can also be duplicate keys (so not a Set or a Map). So it seems to like MessageHeaderList would be better because it is singular like any collection and specifies which kind of collection it is in the name, therefore managing expectations. The only implementation of MessageHeaders looks quite awkward to me as something singular extends something plural: public class HeaderGroup implements MessageHeaders, Serializable This would become: public class HeaderGroup implements MessageHeaderList, Serializable Which seems better to my eye. MessageHeaders does not currently extend List, it could, but we can leave that out as a YAGNI. While we are here, MessageHeaders seems overly wordy in its API name by using "Header" in ALL API names. I think we can do better, especially when you thing more in terms of the List and Map APIs. We currently have MessageHeaders defining: containsHeader(String) countHeaders(String) getFirstHeader(String) getHeader(String) // confusing to have this AND getFirstHeader(), I'd prefer NOT to have this to make it obvious when you call getFirstHeader() that HTTP allows for duplicates. getHeaders() getHeaders(String) getLastHeader(String) headerIterator() headerIterator(String) Which could be renamed to re-enforce the concept of keyed access like a Map (albeit a case-insensitive one which makes me wonder if MessageHeaderMap would not be a better name but I am shying away from that): containsKey(String) // like a Map's containsKey() countKeys(String) getFirst(String) // like a Map's get() but qualified keyArray() // like a Map's ketSet() but returning an array keyArray(String) // like a Map's ketSet() but returning an array getLast(String) // like a Map's get() but qualified keyIterator() keyIterator(String) Removing: get(String) // like a Map but as a above: confusing to have this AND getFirstHeader(), I'd prefer NOT to have this to make it obvious when you call getFirstHeader() that HTTP allows for duplicates. WDYT? Gary On Sat, Dec 14, 2019 at 11:32 AM Michael Osipov <[email protected]> wrote: > Am 2019-12-12 um 10:39 schrieb Oleg Kalnichevski: > > Gary, Michael, > > > > If you feel like renaming things now should be the last opportunity to > > do so, while 5.0 is blocked by HPACK bug discovered by Ryan. > > I have already identified a few things, first PR has been pushed. The > next will follow. > > Michael > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
