On Thu, Dec 3, 2015 at 5:07 AM, Nick Wellnhofer <[email protected]> wrote: > Lucifers, > > Let's continue the Clownfish API review and have a look at the Blob and > ByteBuf classes. > > Blob_compare and ByteBuf_compare can probably go away.
+1 A Lucy test-only module, Lucy::Util::BlobSortEx uses Blob_compare, but it can just reimplement it locally. No need for it to be an officially supported Clownfish API. > Then we should think about whether we need all these methods. The Mimic > methods are essentially just a shortcut for Set_Size(0) followed by Cat. > Cat_* can also be replaced with Cat_Bytes. For example, instead of > BB_Mimic(self, other) one could write: > > BB_Set_Size(self, 0); > BB_Cat_Bytes(self, BB_Get_Buf(other), BB_Get_Size(other)); > > Lucy makes little use of these methods, so it's hard to tell which ones are > the most useful. > > Also, I always found the name "Mimic" a bit weird. What about Assign and > Assign_Bytes? The `Mimic` method originated because I wanted to generalize copying a value without resorting to Clone and the performance penalty of allocating a new object. If I recall correctly, it was used in places like Lexicon (inside of a TermStepper) and SortCache. Terms in Lucy have type Obj, so calling Set_Size and Cat wasn't an option. We needed something that could be used with a numeric type like Float64 in addition to a CharBuf or ByteBuf. The relevant sections in Lucy have been reworked, so there's no justification for Clownfish needing to provide Mimic as far as Lucy is concerned. And I don't think it's a popular concept across most object systems. I'd say zap Mimic and replace any remaining instances with Set_Size/Cat as needed. > Finally, the ByteBuf class and all of its methods can be made public. +1 Marvin Humphrey
