> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato_at_collab.net]
> Sent: woensdag 28 januari 2009 16:16
> To: Bert Huijben
> Cc: 'Greg Stein'; dev_at_subversion.tigris.org
> Subject: Re: svn commit: r35512 - in trunk/subversion: include
> libsvn_client svn
>
> Bert Huijben wrote:
> >> -----Original Message-----
> >> From: Greg Stein [mailto:gstein_at_gmail.com]
> >> Sent: Wednesday, January 28, 2009 12:26 AM
> >> To: dev_at_subversion.tigris.org
> >> Subject: Re: svn commit: r35512 - in trunk/subversion: include
> >> libsvn_client svn
> >>
> >> I disagree with this change.
> >>
> >> You're adding a bunch of complexity in order to save *three*
> >> arguments? That is a very poor tradeoff. Now we have a structure,
> >> constructors, people have to create/init that structure to call the
> >> function, the function will just unpack its values, etc.
> >>
> >> Now, if there were *ten* arguments, then... well, probably still
> "no".
> >> Packing arguments into a struct to be unpacked within the function
> >> generally does not help anything. It just makes it harder to use.
> >
> >
> > Hi,
> >
> > This specific example was discussed on this list a few months ago as
> a
> > specific example when to introduce an args structure. The work just
> wasn't
> > completed before.
> >
> > See: [RFC] Introducing svn_client_make_diff_args? (Was: ...)
> > http://svn.haxx.se/dev/archive-2008-09/0674.shtml (Google reference,
> but url
> > is down right now)
> >
> > We are on svn_client_status4 right now, with only 6 point releases,
> > svn_client_log5, svn_client_diff5.. Let's go to svn_client_status99
> then and
> > let all 98 others be deprecated but be part of our code before 2.0.
> >
> > Providing functions with dozens of Boolean arguments doesn't give us
> a clean
> > api. Using an args object with a good set of defaults makes the
> choices
> > explicit in code and commit logs without referring to the help (or
> > intellisense of your favorite editor) to see what the seventh Boolean
> is
> > about.
> >
> > See also the forwarding of the args inside the status call to the
> next call
> > for the externals. It reduced extra arguments there again.
>
> Bert, I gotta say I that I, too, disapprove of this change, and
> absolutely
> do not want to go down this path. The closest to this that I would
> agree to
> go would be to combine all the booleans into a field of bitflags. But
> generic structures of function args are, as Greg as noted, just a lot
> of
> pain for precious little gain.
>
> As for svn_client_status99() ... that's a tad over-dramatic, don't you
> think?
They give an initial pain, but they document exactly what the users ask from an API (Out of your memory: What is the third boolean passed to svn_client_status3 ?) and will need no further updating later on when new options are added that have a logical default.
(In 99% of the cases the logical default is just what svn would do without any arguments.. as you don't have to think twice about that default)
I just came to 74 SVN_DEPRECATED methods in svn_client.h, most of which could have been avoided by providing a little bit of extensibility via either a bitflags parameter or an args object. (35% of the file is filled with deprecated methods.. and this is of course after trimming their comments when rev'ving).
And svn_status is now the third/fourth method to provide an args object. With three options (I decided not to add the update argument, (I did add it to the args object in SharpSvn).. but most depth parameters are on the args object there too).
Which others would you like to revert too?
Feel free to revert..
svn_client_status4: r35518,r35515-r35514,r35512
svn_client_log5: r35242,r35245
Adding args objects is not the normal way to code for extensibility in C, but for .Net libraries it is explicitly documented as the way to design libraries for future extensibility and it as added benefit it works to get better readable/maintainable code. (Google can certainly give you some references to the class library design guidelines and the reasoning behind this and related rules).
It's better maintainable than default values on functions calls, as used in many programming languages; as you can never change the default value of these default values. (E.g. In C++ the default is compiled in at the caller; so it becomes part of your ABI)
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1063558
Received on 2009-01-28 17:16:04 CET