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
Received on Fri Jan 26 08:59:25 2007