Re: STL algorithm member function problem
Alberto Ganesh Barbati wrote:
lists@givemefish.com ha scritto:
class Rule : public std::unary_function<std::string, bool> {
public:
virtual result_type operator()(const argument_type&) const =0;
};
typedef boost::shared_ptr<Rule> RulePtr;
///////////////
// Concrete rules are inherited from the Rule class and define
// the operator()
.....
vector<string> fileList;
// .... fill fileList with a series of file names ...
//Now remove items from the fileList according to the rules.
//Iterate over all the rules in the rules vector. Apply the rule
to each of
// items in the fileList and remove it if the rule returns true.
for(vector<RulePtr>::const_iterator ruleIter = m_rules.begin();
ruleIter != m_rules.end(); ++ruleIter) {
// This is the problem line...
std::remove_if(fileList.begin(), fileList.end(), ptr_fun(
xxx );
}
This code suffer of a common but very serious bug. But let's answer the
question first and the bug later.
If Rule wasn't a polymorphic class, the correct answer would be the
simplest one:
std::remove_if(fileList.begin(), fileList.end(), *ruleIter);
Supposing, of course, that he changed the vector to contain
Rule, and not RulePtr.
However in your case it won't work, because the static type of *ruleIter
is Rule which is an abstract class which can't be passed by-value to
remove_if. Notice that making Rule non-abstract doesn't help because of
the slicing problem: as it's passed by-value the dynamic type is lost.
Passing by-value and polymorphism are things that don't go together
well. Functors and predicate are usually passed by-value, so you should
have realized by now that having a polymorphic predicate wasn't a good
idea in the first place.
Which of course could be considered a major design flaw.
Of course, it's possible to make value based objects behave
polymorphically, by means of the letter-envelope idiom.
What I suggest is to change the design like this:
class RuleBase {
public:
virtual bool operator()(const std::string&) const =0;
};
class Rule : public std::unary_function<std::string, bool> {
boost::shared_ptr<RuleBase> rule;
public:
Rule(const boost::shared_ptr<RuleBase>& r) : rule(r)
{}
// nonvirtual!
result_type operator()(const argument_type& s) const
{
return (*rule)(s);
}
};
Of course now all your rules shall inherit from RuleBase, not Rule.
That's a sort of a variant of the letter/envelope idiom. In a
case like this, where the interface is extremely narrow, it may
even be preferable (just as using shallow copy instead of deep
works if the objects are inmutable). The letter/envelope idiom
has the advantage that you don't need to duplicate the interface
in two separate, formally unrelated classes. (It also has the
advantage that it is an established pattern, immediately
recognizable as such. And the disadvantage that if you forget
to override one of the virtual functions in a derived class, the
compiler won't complain, since the virtual functions in the base
are not pure virtual. The code will core dump rather quickly,
however.)
[...]
However, if you have a lot of strings and a lot of predicates, the
performance of this loop are suboptimal because strings are moved a lot
of times. It is much better IMHO to move the loop *in the predicate*, as in:
The loop over the rules, you mean.
Independantly of the performance issues, I'd favor this from the
design point of view. You apply *one* rule, which just happens
to be a composite rule. You can even make the solution more
complex, with and's and or's of the different rules; his case is
just a degenerate OR node:
while ( current != end && !(*current)( s ) ) {
++ current ;
}
return current != end ;
For and:
while ( current != end && (*current)( s ) ) {
++ current ;
}
return current == end ;
For completeness, let's not forget not:
return ! (*current)( s ) ;
Obviously, you can put OR nodes in the list of an AND, and vice
versa. Add an REMatch node, based on boost::regular_expression,
and you can create just about any condition imaginable.
(In fact, it's probably possible to construct just about any
rule with a single regular expression. But the expression
becomes extremely complex, and boost::regular_expression
requires it to be built up as a string. My own
RegularExpression class allows the or'ing of already constructed
regular expressions, and converts the results to a DFA on the
fly, so it would be much faster than the looping constructs
above, but it only handles pure regular expressions, and none of
the extensions that are almost always present elsewhere.)
--
James Kanze (GABI Software) email:james.kanze@gmail.com
Conseils en informatique orient?e objet/
Beratung in objektorientierter Datenverarbeitung
9 place S?mard, 78210 St.-Cyr-l'?cole, France, +33 (0)1 30 23 00 34
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]