<br><br><div class="gmail_quote">On Fri, May 18, 2012 at 1:36 AM, Marius Tibeica <span dir="ltr"><<a href="mailto:mtibeica@gmail.com" target="_blank">mtibeica@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><br><div>The question now is, what's next?</div><div>Should I start implementing other methods?<br></div></div></blockquote><div><br>Re latest code...<br><br><br>Commit message format:<br> [add|fix|clean]: filename method (, ...) (, filename ...) - what was added/fixed/cleaned<br>
<br> add - create new functionality<br> fix - repair existing functionality<br> clean - no change in functionality<br> If a LOT of methods are affected, the method list can be truncated to a few key ones<br><br> No need to change the messages you've already committed.<br>
<br><br>Should we create a single node-xapian.h header file? (Which I guess could include the contents of op.h)<br><br><br>Code style:<br> if/else and function opening brace on same line! (see op.h macro)<br> local vars begin with letter a, to differentiate from function arguments<br>
<br>
<br>DECLARE_POOLS<br>
<br>
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.<br><br> 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()<br>
<br>
The _do functions don't need a HandleScope, since the caller has one.<br>
<br> 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<br><br><br>Enquire::GetMset:<br><br> Remove try/catch around "new GetMset_data...", which no longer throws since it's not derived from AsyncOp!<br>
...<br> bool aAsync = args.Length() == 3 && args[2]->IsFunction();<br> if (args.Length() < 2 || args.Length() > +aAsync+2 || !args[0]->IsTypename() || ... )<br> return ThrowException(...);<br> ...<br>
Local<Value> aResult;<br> try { /// if we pull the try/catch from the _do functions<br> aResult = aAsync ? GetMset_async(...) : GetMset_sync(...); /// or GetMset_do(aAsync, ...)<br> } catch (Local<Value> ex) {<br>
return ThrowException(ex)<br> }<br> return scope.Close(aResult); /// it's not necessary to Close() for Undefined() but prob not harmful<br><br>Cheers,<br><br>Liam<br></div></div>