[Coin-discuss] OsiCuts bug

[Laszlo Ladanyi]

The first one definitely looks like a bug. I looked at those iterators and it
looks they could use a thorough review/revision. They are not bad, just their
usage is different from the usage of the iterators in the STL (like not being
able to create an OsiCuts::iterator without having an OsiCuts object).

The second is a good question... I guess the base class' op== should really be
called 'SameEffectiveness' or something. == is usually used when the objects
are the same. Or when they are equivalent... in which case the current
definition is fine... I don't know what's best. However, it's clear that the
meaning of == is different in the base class and the derived class, which is
bad. So perhaph renaming the base class' == would be best. But at the same
time there probably should be a virtual op== in the base class, just it should
not be defined (or defined as returning false). And the derived class should
have, as you suggest, an op== taking an OsiCut& as argument. Yes, I think this
is the best. So I propose:

1) rename current op== to soemthing in the base class
2) have op== return false in the base class
3) add op==(const OsiCut&) to the derived class (simple routine: does a
   dynamic cast, if succeeds then calls the other op== otherwise returns
   false)

Unless someone complains I'll make these changes early next week.

--Laci

On Sat, 24 May 2003, Matthew Galati wrote:

> Hi Folks,
> 
> 1 bug, 1 design question to report on OsiCuts/OsiCut:
> 
> 
> 1.)
> OsiCuts::iterator OsiCuts::iterator::operator++() {
>     if ( (rowCutIndex_+1)>=cutsPtr_->sizeRowCuts() ) {
>         ...
>         if ( cuts_.sizeRowCuts() > 0 && colCutIndex_<cuts_.sizeRowCuts() )
> 
> Seems it should be checking the colCutIndex against the number of col 
> cuts, not row cuts
> 
>         if ( cuts_.sizeRowCuts() > 0 && colCutIndex_<cuts_.sizeColCuts() )
> 
> 
> 2.)
> Although OsiCut::operator== is virtual, the derivation in OsiRowCut is 
> not actually overloaded...
> 
> inline virtual bool operator==(const OsiCut& rhs) const;
> bool OsiCut::operator==(const OsiCut& rhs) const
> {
>   return effectiveness()==rhs.effectiveness();
> }
> 
> bool OsiRowCut::operator==(const OsiRowCut& rhs) const
> 
> Since the arguments are different... to overload it, the operator should 
> be defined as
> 
> bool OsiRowCut::operator==(const OsiCut& rhs) const
> 
> This is perhaps intentional as, the OsiRowCut::operator== needs to 
> access members of the derived class that don't exist in the base 
> class... this actually brings up the question of whether or not 
> OsiCut::operator== should be virtual in this case. This example came up 
> in a situation where we are looping over OsiCuts with the 
> OsiCuts::iterator... once getting the OsiCut, we want to apply the 
> derived version of the operator==, but the current design will use the 
> base classes operator== (despite it being virtual). To get around this, 
> we have to dynamic_cast the return of the iterator to an OsiRowCut 
> before using operator==. I am not sure if this can work - but can the 
> iterators be virtual so that we are not forced to cast?
> 
> Matt
> 
> 
> -- 
> Matthew Galati
> ISE Lehigh University
> IBM Service Parts Solutions
> 610.758.4042 (Office)
> 610.882.0779 (Home)
> magh at lehigh.edu, magal11 at us.ibm.com
> http://sagan.ie.lehigh.edu/mgalati/
> 
> 
> _______________________________________________
> Coin-discuss mailing list
> Coin-discuss at www-124.ibm.com
> http://www-124.ibm.com/developerworks/oss/mailman/listinfo/coin-discuss
>