zorba-coders team mailing list archive

[Matthias Brantner <matthias.brantner@xxxxxxxxxx>]

Review: Needs Fixing

- dynamic_context::get_environment_variable(const zstring varname)

should be

dynamic_context::get_environment_variable(const zstring& varname)

- spaces before "(". for example "if (...) instead of if(...)"

- all new functions in sequences_impl.cpp are indented with two spaces. the other functions are not

- if(result)
{
  STACK_PUSH(true, state);
}
STACK_PUSH(false, state);

could be written as STACK_PUSH(result!=NULL, state);

- In FnEnvironmentVariableIterator::nextImpl, the check  if(consumeNext(item, theChildren[0].getp(),planState)) is not necessary. consumeNext always returns true because the functions is defined to accept xs:string (not xs:string?)

- item->getString() returns a const zstring&. This should be the type of the variable.

- store::Item_t item2 is not used in the function

- In FnAvailableEnvironmentVariablesIterator::nextImpl, the line a = result->getStringValue().c_str(); and the variable a don't seem to be necessary

- The environment-related functions should be specified using

<zorba:methods> 
  <zorba:accessesDynCtx returnValue="true"/>                                                      
</zorba:methods>

- All the lines after 14.8.5 fn:unparsed-text  have windows line endings

- you can check using transcode::is_necessary if a encoding is supported

- It's unclear when the original stream for unparsed-text is released. This seems to be a memory leak. 

- The unparsed-text related functions should be specified as

<zorba:methods> 
  <zorba:accessesDynCtx returnValue="true"/>                                                      
  <zorba:isSource returnValue="true"/>                                                            
</zorba:methods>

- What's the default encoding for unparsed-text?
-- 
https://code.launchpad.net/~zorba-coders/zorba/fn_envvars/+merge/88143
Your team Zorba Coders is subscribed to branch lp:zorba.