← Back to team overview

zorba-coders team mailing list archive

Re: [Merge] lp:~zorba-coders/zorba/ft-base64Binary into lp:zorba

 

Review: Approve

include/zorba/item.h: Docs do not say what happens, if 
getBase64BinaryValue is called when the item is streamable.

include/zorba/item_factory.h: "whence" isn't the most commonly used word 
(http://en.wiktionary.org/wiki/whence) maybe we could use another one

src/runtime/base64/base64_impl.cpp:43 is it ok to ignore the result of 
consumeNext because the input can't be an empty sequence? If so, should 
we add a comment?

Could we implement Base64BinaryItem::getStringValue() using 
Base64BinaryItem::getStringValue2(zstring& val)?

Does Base64BinaryItem::getStringValue2(zstring& val) work if val is not
empty? It seems that theValue would be prepended to the current content
of val. Could we implement this method by emptying val and calling 
Base64BinaryItem::appendStringValue on it?

StreamableBase64BinaryItem::materialize(): 
- Is there a reason for the number 4048?
- In general those might be a lot of re-allocations, if the item is a 
  little larger (e.g. if the item is 1MB, we've got more than 250 
  re-allocations). I'm not sure where the right trade-off is, but it 
  seems that this parameter might become expensive (if someone 
  materializes a large item that should be streamed …)

I just hope that src/zorbaserialization/zorba_class_serializer.cpp works.

-- 
https://code.launchpad.net/~zorba-coders/zorba/ft-base64Binary/+merge/94081
Your team Zorba Coders is subscribed to branch lp:zorba.


Follow ups

References