On Tue, Nov 1, 2016 at 8:47 PM, Alex Rousskov <rouss...@measurement-factory.com> wrote: > On 11/01/2016 02:02 PM, Kinkie wrote: >> the attached patch extends SBufContainerJoin to have prefix and >> suffix arguments. This can support a use-case which I found in the >> current ACLRegexData work I'm following, where we need to transform >> {"foo", "bar", "gazonk"} >> into >> (foo)|(bar)|(gazonk) >> >> It can be done with the current version of the SBufContainerJoin >> function, but it requires one more copy. >> There are probably further cases of this pattern elsewhere, all hand-rolled. > > I agree that the use case is legitimate, but the implementation does not > solve the underlying cause -- extra copies.
It removes the need of one copy in a common use case, but it can do better. It also has the drawback of iterating over a full container; reworking to add a target SBuf and to use begin-end iterators instead of a container. > I recommend reworking this by adding a dest parameter: > > /// copies all container items (delimited by separator) into dest > ... > /// \returns dest > SBuf &ContainerToSBuf(SBuf &dest, ...); > > and using that function to implement the SBufContainerJoin() or a > similarly named convenience wrapper that does not require a dest > parameter and that returns a copy of an internally used SBuf. Can simply return the modified SBuf. All the convenience without the bloat. Also taking the argument by copy; will cow if needed (passing by reference makes for awkward client code as it'd have to be an lvalue). So the common pattern would be: auto out = JoinContainerIntoSBuf(SBuf(), container.begin(), container.end(), SBuf("separator")) > In either case, please do not forget to document that the new prefix and > suffix are always added, even if the container is empty. Done. >> // sz can be zero in two cases: ... or all items are zero-length. > > FWIW, this is a lie. Zero-length items do not result in zero sz unless > the separator itself is also surprisingly empty. Zero-length items is > not a special case worth mentioning IMO. The zero-size check should be > replaced with a direct (begin != end) check to avoid complexity and > confusion -- it is only needed to avoid dereferencing begin() AFAICT. Ok. > HTH, Sure does :) Updated patch attached. -- Francesco
sbufcontainerjoin-v2.patch
Description: Binary data
_______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev