On Sat, Aug 24, 2002 at 10:10:12AM -0700, Brent R. Matzelle wrote:
> To download the API download the source from the RapidSVN repository
> (under /src/svncpp):
>
> http://svn.collab.net/repos/svn/clients/rapidsvn/trunk
Hmm. I'm concerned that by using this API you lose access to the
svn_error_t structure. Most of the functions simply return a true
or a false. That's completely useless to me as a developer. I
want to know *why* it failed and I think this API loses that.
I think I'd have to call GetError after a function returns false.
Ick.
In a true C++ API, I would expect the errors to be handled via
exceptions rather than a bool true/false value. The concept of a
global value called Err is frightening to me. Errors in C++ ought
not to be handled that way. (Please don't say that exceptions are
slow - if that's a concern, then don't use C++.)
For example, Status::LastChanged should return an unsigned long
(given that SVN does not have negative revision numbers). On an
error, it might throw SVNEntryNotVersioned exception or something
like that.
I also would consider that the pool should be passed to the
constructor of each class. I'm not entirely happy that it does a
svn_pool_create(NULL). The issue is that pools are hierarchical and
you're going to lose that aspect. At the very least, allow for a
constructor where the pool parameter is passed in.
And, the hungarian notation is, well, antiquated. Not to mention
some of the code formatting is a bit weird (what's up with
Status::LoadPath?)
My curmudgeonly $.02 off a cursory code review. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Aug 24 20:19:57 2002