[Debtags-devel] Hello, and a few improved tag descriptions

[Mike Paul]

--=-08PswX2ykg0DPeb5pPol
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

(Argh!  I did it again.  I really must find a way to get my mail client
 to automatically use "reply to list" rather than "reply to sender"
 when replying to messages from the list.
 Apologies again, Enrico, for the duplicate message.)

On Sun, 2005-06-05 at 15:11 +0200, Enrico Zini wrote:
> Formally this is true; however, using the first three avoids to build
> a std::set (or OpSet) structure when it's not needed.  But maybe I'm
> trying to optimize too early in the development?

Probably premature optimization; I don't think constructing a std::set
is a particularly expensive operation, and maintainability of the source
code is more important.  Consider this snippet from debtags.cc:

        // If the package is really untagged, there's nothing we can do her=
e
        virtual void consume(const std::string& item) throw () {}
        virtual void consume(const OpSet<string>& items) throw () {}
       =20
        // Catch the tags that belong to _item
        virtual void consume(const std::string& item, const OpSet<string>& =
tags) throw ()
        {
                for (OpSet<string>::const_iterator i =3D tags.begin(); i !=
=3D tags.end(); i++)
                        checkTag(*i);
        }
       =20
        // Catch the tags that belong to _item
        virtual void consume(const OpSet<string>& items, const OpSet<string=
>& tags) throw ()
        {
                for (OpSet<string>::const_iterator i =3D tags.begin(); i !=
=3D tags.end(); i++)
                        checkTag(*i);
        }

The first two functions don't do anything but still have to be
documented as to why they don't, and the second two are identical except
for the type of a parameter which isn't used in the function body.  I
think the first three functions here should be "forwarding" functions,
possibly inline, which construct OpSets as necessary and then call the
fourth function.  (In the case of the first two versions, the "tags"
parameter will be an empty set, so the for-loop will do nothing.)  These
forwarding functions, being generic, can be defined in the Consumer
template class rather than having to be rewritten in every subclass.

> /me also starts considering getting rid of OpSet and using algorithm
> functions instead of overridden operators.  However, I'm quite attached
> to being able to say:
>=20
>   ts +=3D (ts - ts1)
>=20
> Instead of something like:
>=20
>   set_difference(ts.begin(), ts.end(), ts1.begin(), ts1.end(), back_inser=
ter(temp));
>   copy(temp.begin(), temp.end(), ts);
>  =20
> This is another big doubt of mine: I hate OpSets because they're just
> std::set with some operators overridden, and when one sees them it's
> hard to figure out what the heck they are; however, the whole Debtags
> code is full of set operations, and they make it all so easier to read.

It's a choice between two different kinds of readability, I guess:
readability in the sense of clear and concise notation, or readability
in the sense of using well-known standard operations with descriptive
names.  I can't think of any strong reason to prefer one over another,
so I'd just leave it the way it is unless someone else can come up with
a reason to change.  (However, if you also use standard algorithms
directly, which aren't covered by the operators, then you should think
about whether you're mixing different levels of abstraction.)

BTW, I'm curious why apparently all the classes' member functions are
declared "throw ()".  That means that basically any exception thrown
anywhere will result in a call to std::unexpected() which terminates the
program with no chance of catching and handling the problem.  It's
probably best to leave off the exception specification in most cases,
and only specify it on functions where you're *sure* of the set of
possible exceptions, and where it isn't likely to change in the future.
--=20
Mike Paul <mike@wyzardry.net>

--=-08PswX2ykg0DPeb5pPol
Content-Type: application/pgp-signature; name=signature.asc
Content-Description: This is a digitally signed message part

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQBCozaj3SZkqUhyWy4RAgY8AKC+cWLZi+LyVm4N9NsUd0A9+ClabACdFFRW
ttlsg7ytFbNMyaUhyGvwgBU=
=msDJ
-----END PGP SIGNATURE-----

--=-08PswX2ykg0DPeb5pPol--