Re[2]: [lmi] Naming certain overloaded functions

[Vadim Zeitlin]

On Thu, 26 Oct 2006 15:20:24 +0000 Greg Chicares <address@hidden> wrote:

GC> I thought the problem was this:
GC> 
GC> struct Base {
GC>   virtual foo(int);
GC>   virtual foo(double);
GC> };
GC> 
GC> struct Derived: Base {
GC>   virtual foo(int); // Now Base::foo(double) is hidden.
GC> };

 Yes, exactly.

GC> I had taken it as a theorem that this problem can be avoided by
GC> implementing all virtuals in the derived class;

 Yes, it can. But it's IMHO better not to avoid the problem at all by not
having it in the first place.

GC> and as a corollary that it can be avoided by making all virtuals pure
GC> in the base class.

 I don't think such considerations should decide whether a virtual function
is or not pure virtual. This is a design decision and shouldn't be taken
to accommodate C++ quirks.

GC> We can change that base class. I see you post an idea below, so
GC> let me sketch mine here before I examine yours:
GC> 
GC> class streamable {
GC>   public:
GC>     void write(xml_lmi::Element&) const; {/* calls do_write() */}
GC>   private:
GC>     virtual void do_write(xml_lmi::Element&) const = 0;
GC> };
GC> 
GC> class Ledger: public streamable {
GC>   private:
GC>     virtual void do_write(xml_lmi::Element&) const {/* implement here */}
GC> };

 Yes, this could be conceptually more elegant but I agree that it's
probably not worth the extra hassle for such a simple base class.

GC> means not that you wish Base::write() were nonvirtual, but
GC> rather that Base::write() is not overloaded, and shouldn't be.
GC> 
GC> Am I understanding this correctly now?

 This is what I thought when I wrote my first reply, avoiding overloading
seemed like the best way to solve the problem in this particular case.

GC> [reordered]
GC> > But then
GC> > in this case it just doesn't seem like a good idea to have the 
std::ostream
GC> > overload anyhow: the 2 functions do quite different things AFAICS so it's
GC> > confusing that they have the same name.
GC> 
GC>     virtual void write(xml_lmi::Element&) const;
GC>             void write(std::ostream&    ) const;
GC> 
GC> They both write *this to somewhere. One writes to an xml element;
GC> the other, to a stream. Why is that a bad use of overloading?

 I was apparently too categorical. Looking more carefully, it seems that
there is indeed more similarities between these methods than I thought.
Still, I think the question to ask should be "do we really gain much by
overloading write() here?" instead of "why shouldn't we overload it?".
It just seems simpler to name write(ostream&) something else...

 But then it's probably also true that streamable class interface is not
going to change often (or ever) and no more write() overloads are going to
be added and so the current code can be also kept without problems (with
just do_write() renamed to really_write() or whatever), finally.


GC> >  Finally, completely unrelated to the overloaded functions names, but 
still
GC> > related to this do_write() function, I'd like to also change its
GC> > parameters. This is my pet peeve so please bear with me if you heard it
GC> > before (Evgeniy definitely did) but using bool parameters for things other
GC> > than obvious on/off switches is simply evil, please see my attempt to
GC> > explain why here:
GC> > 
GC> >   http://www.wxwidgets.org/develop/standard.htm#no_bool_params
...
GC> I completely agree.

 Then the only question is: should we change this right now or wait for
after the merge? I'd prefer to fix it a.s.a.p. while this discussion is
still fresh in our minds if possible.

GC> > So I strongly propose that do_write() signature be changed to
GC> > 
GC> >   enum xmlStyle
GC> >   {
GC> >           xmlStyle_Light,
GC> >           xmlStyle_Full
GC> >   };
GC> > 
GC> >   void do_write(xml_lmi::Element&, xmlStyle style) const;
GC> 
GC> I embrace the principle, but would ask you to write it this way:
GC> 
GC> enum enum_xml_style
GC>     {e_xml_light
GC>     ,e_xml_heavy
GC>     };

 Oops, sorry about this, certainly. I just got carried away by my wx
mindset after (re)reading the link above. It was meant as a conceptual
example, not to be copied verbatim into LMI code.

 Thanks,
VZ