[Xapian-devel] GSoC xapian node binding

[Liam]

On Fri, May 18, 2012 at 1:36 AM, Marius Tibeica <mtibeica at gmail.com> wrote:

>
> The question now is, what's next?
> Should I start implementing other methods?
>

Re latest code...


Commit message format:
  [add|fix|clean]: filename method (, ...) (, filename ...) - what was
added/fixed/cleaned

  add - create new functionality
  fix - repair existing functionality
  clean - no change in functionality
  If a LOT of methods are affected, the method list can be truncated to a
few key ones

  No need to change the messages you've already committed.


Should we create a single node-xapian.h header file? (Which I guess could
include the contents of op.h)


Code style:
   if/else and function opening brace on same line! (see op.h macro)
   local vars begin with letter a, to differentiate from function arguments


DECLARE_POOLS

  In the _do functions, perhaps the try/catch should move to the caller,
which would ThrowException() and delete aData on catch? (The only throw is
in AsyncOp::AsyncOp.) I'm not sure of the implications of returning
ThrowException to scope.Close() -- may be harmless; something to look into.
Even so, it's probably clearer to ThrowException from the caller. Also
there's no need to delete x when x=new X() throws, I believe. And if
_process() returns an error, you'd need to convert it to an
Exception::Error and throw that  for the caller.

  Does _process() need to return a new Xapian::Error, or can it return the
Error that was caught? I was doing new there previously to attach the error
to the GetMset_data object. Of course then you'd need to do the new in
_pool()

  The _do functions don't need a HandleScope, since the caller has one.

  Did you look at merging the two _do functions? If not mergeable, then we
prob don't need _do in the names, just _sync & _async


Enquire::GetMset:

  Remove try/catch around "new GetMset_data...", which no longer throws
since it's not derived from AsyncOp!
  ...
  bool aAsync = args.Length() == 3 && args[2]->IsFunction();
  if (args.Length() < 2 || args.Length() > +aAsync+2 ||
!args[0]->IsTypename() || ...  )
    return ThrowException(...);
  ...
  Local<Value> aResult;
  try { /// if we pull the try/catch from the _do functions
    aResult = aAsync ? GetMset_async(...) : GetMset_sync(...); /// or
GetMset_do(aAsync, ...)
  } catch (Local<Value> ex) {
    return ThrowException(ex)
  }
  return scope.Close(aResult); /// it's not necessary to Close() for
Undefined() but prob not harmful

Cheers,

Liam
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.xapian.org/pipermail/xapian-devel/attachments/20120522/d2abbfa5/attachment.htm>