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

[Benjamin Mesing]

Hello

> (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.
This happens to me all the time :-)

> 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:
I am not sure I agree here. Though generally I prefer writing readable
code, the consume operations are called very often which makes each
overhead accumulate. But I am not sure about the overall impact here.
Some measurements should help, but who has time for this kind off
stuff...?


>         // If the package is really untagged, there's nothing we can do here
>         virtual void consume(const std::string& item) throw () {}
>         virtual void consume(const OpSet<string>& items) throw () {}
>         
>         // 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 = tags.begin(); i != tags.end(); i++)
>                         checkTag(*i);
>         }
>         
>         // 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 = tags.begin(); i != 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.
Enrico do you actually call the consume(const OpSet<string>& items,
const OpSet<string>& tags) function? Only asking out of curiousity.

> > /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:
> > 
> >   ts += (ts - ts1)
> > 
> > Instead of something like:
> > 
> >   set_difference(ts.begin(), ts.end(), ts1.begin(), ts1.end(), back_inserter(temp));
> >   copy(temp.begin(), temp.end(), ts);
>   
> > 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.
Please don't drop the OpSet. I use it excessively in packagesearch
because it is so convenient to use. However perhaps one could extract an
interface which defines the operators as virtual (are virtual operators
allowed in C++)? This would avoid the dependency on libtagcoll when
using the OpSet only, but probably it is not worth the effort.

And for "it's hard to figure out" part: You could always document their
purpose :-) Eclipse/Java would be so helpful here with hinting what the
type is supposed to do!


> 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.
:-) This is something I complained about to Enrico some time ago. I
already had those std::unexpected exceptions! Perhaps together we have
the power to shift his attention towards this point ;-)

Greetings Ben