Rev-interfaces to support optional parameters

[Nik Clayton <ni...@ngo.org.uk>]

Folks,

How controversial would it be to rev some API interfaces to better deal with 
the possibility of optional parameters?

For example, consider svn_client_log3().  The prototype for this is:

svn_error_t* svn_client_log3(
     const apr_array_header_t *   targets,
     const svn_opt_revision_t *   peg_revision,
     const svn_opt_revision_t *   start,
     const svn_opt_revision_t *   end,
     int   limit,
     svn_boolean_t   discover_changed_paths,
     svn_boolean_t   strict_node_history,
     svn_log_message_receiver_t   receiver,
     void *   receiver_baton,
     svn_client_ctx_t *   ctx,
     apr_pool_t *   pool
);

Due to some of the magic that happens in the swig bindings, the 
receiver_baton, ctx, and pool arguments are all optional when called from a 
Perl app.

I'd like to make peg_revision, limit, discover_changed_paths, and 
strict_node_history optional as well (in the Perl bindings).  peg_revision 
would default to svn_opt_revision_unspecified, the others would default to 
0.  But I can't do that because receiver isn't optional.  peg_revision 
appears early in the list, and once you introduce an optional parameter in 
to an argument list all the following arguments need to be optional too.

So I'd like to create svn_client_log4, which would look like this:

svn_error_t* svn_client_log4(
     const apr_array_header_t *   targets,
     const svn_opt_revision_t *   start,
     const svn_opt_revision_t *   end,
     svn_log_message_receiver_t   receiver,
     void *   receiver_baton,
     int   limit,
     svn_boolean_t   discover_changed_paths,
     svn_boolean_t   strict_node_history,
     const svn_opt_revision_t *   peg_revision,
     svn_client_ctx_t *   ctx,
     apr_pool_t *   pool
);

That puts all the mandatory arguments at the start of the list, and groups 
the optional ones at the end, hopefully in the order that they're most 
likely to be used (i.e., most likely that limit will be used before 
discover_changed_paths, strict_node_history, and peg_revision, respectively.

Since SWIG can't wrap macros (at least, the documentation says it ignores 
them) log4() could either be a thin wrapper around log3 in all cases, or we 
could do something like this:

   #if RUNNING_SWIG   /* Will need to be defined in the build system */
   svn_error_t* svn_client_log4(...)
   {
       return svn_client_log3(/* with args in a different order */);
   }
   #else
   /* Short param names to save space -- a proper patch would spell them
      out in full */
   #define svn_client_log4(t, st, en, r, rb, l, dcp, snh, pr, c, p) \
     svn_client_log3(t, pr, st, en, l, dcp, snh, r, rb, c, p);
   #endif

So the C API gets a macro, and doesn't need to pay for an additional 
function call, while the bindings have that small extra cost.

An alternative would to have the Perl bindings expect the arguments to 
log3() to be in a different order.  But I think that's a bad idea, since it 
means that we can't leverage the Doxygen docs, and is likely to lead to 
programmer confusion.

Thoughts?

N

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Rev-interfaces to support optional parameters

[John Peacock <jp...@rowman.com>]

Nik Clayton wrote:
> How controversial would it be to rev some API interfaces to better deal 
> with the possibility of optional parameters?

I'd be inclined to not rev the API, since it will just produce code 
churn with very little gain for consumers other than SWIG.  Just make 
the positional arguments in Perl take 'undef' to mean 'take default' (if 
that isn't already the case).

I suspect that once you finish amending the Perl interface to take named 
parameters, no one will want to use the positional parameters any 
longer, other than existing code (like SVK) which would be broken by 
reordering the parameters anyways.

My 2 cents...

John

-- 
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD  20706
301-459-3366 x.5010
fax 301-429-5748

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org