New struct for holding (url, rev, repo-root) coordinates in client merge code
From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Sun, 25 Mar 2012 17:17:41 +0100 (BST)
Subversion contains some pretty high level code that's pretty inscrutable. I keep asking myself how we would do <whatever> if we were writing in a higher level or more object-oriented language.
In the merge code we work with <url,rev> coordinates a lot of the time. Passing these as two separate variables works OK as long as there are only two and not used too many times. In this code, however, we end up passing them around very many times. We end up with code like this:
Sometimes we have more than two variables because we also want the repository root URL, either to check it's the same repo as some other location or to derive the repo-root-relative path (or fspath). It's reached the point where it's definitely worth having a struct that contains <url, rev, repo_root>. Now we can write things like:
It's not just a case of one parameter
So, I introduced this structure in libsvn_client/merge.c recently:
/* A location in a repository. */
It's not ideal as it is. The pointer to a url_uuid_t sub-structure is unnecessary at this stage
For small bits of code, use of a structure such as this -- with its own 'create' and 'dup' functions -- would be over-complex. However, for the large amounts of code in relatively high level client-library functions, use of it makes a significant contribution to writability and readability.
I'm planning to:
* Replace the 'repo' field with a simple 'const char *repo_root_url' (and maybe a uuid as well), thus simplifying initial allocation and the 'dup' function.
* Extend the visibility to make it an "svn_client__" prefixed module-scope type in libsvn_client, and use it to simplify the interfaces of other private svn_client__ functions.
If it works well there, we can consider making it more global.
This is an archived mail posted to the Subversion Dev mailing list.