Eric Wong <normalper...@yhbt.net> writes:
> Perhaps we can depend on the URI.pm module? It seems to be
> widely-available and not be a significant barrier to installation. On
> the other hand, I don't know its history, either (especially since we're
> now dealing with SVN changes...).
> Anyways, I don't like relying on operator overloading, it makes code
> harder to read and review.
I think code that uses operator overloading, when printed in a
textbook, cast in stone and makes the reader aware that it is never
going to change, is indeed "easy" to read through. But I suspect
that it may be merely giving a false illusion that it is easy to
The problem is that use of such obscure overloading tends to hurt
maintainability. If the initial version Michael produces converts
all the external strings into instances of CanonicalizedPath class,
according to the "convert as early as possible" principle, you can
be assured that all "eq" you see are about the normalized strings
the svn library wants to see, and that may allow us sleep safely.
But the real problem begins six months down the road, when somebody
wants to add a new codepath that reads a new string from an external
source (e.g. perhaps you add a new configuration variable that
specifies a path in the svn repository and does something special
when that path is touched by a revision; the exact nature of the new
feature does not matter in this discussion). The new code can
forget to follow the "convert early" principle, and pass a bare
string read from the configuration around.
A comparison between such a new string and another variable that
holds path that comes from the existing codepath (i.e. Michael's
initial code that perfectly follows the "convert early" principle)
will still use the overloaded eq in "$new_str eq $old_path", thanks
to the language rule of Perl (namely, even though the new string is
a non object, the other side is still an instance of the class).
When the code needs to compare two or more such "new" strings (e.g.
perhaps it wants to remove duplicates from the set of paths it reads
from the configuration), however, "eq" silently turns back to a
simple string comparison, as "$new_1 eq $new_2" will not magically
turn into "Canonicalize($new_1)->cmp(Canonicalize($new_2))".
This kind of error is unnecessarily hard to catch mostly because the
previous "$new_str eq $old_path" does work; it masks the problem.
Overloading of "eq" is making it harder to spot new bugs.
If the code never uses "eq" to compare canonicalized paths, and all
the surrounding code compare paths using explicit method call on
objects, it makes it crystal clear to the readers that paths held in
a bare string is unwelcome in the codepath. It makes it harder to
add new code that uses and passes around a bare string by mistake to
such a codepath, I would think.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html