Subject: Re: commit: UT_String (longish)
From: Mike Nordell (tamlin@algonet.se)
Date: Sat Jun 02 2001 - 21:47:42 CDT
I hope I'm not (too) rude in this comment, it's not my intention.
> > There's no way we're gonna carry around a vftable in UT_String. No way!
> > Neither should it inherit any other class, especially not one with a
> > vftable of its own.
>
> This is flawed thinking.
No it's not.
> Just because a class was originally designed to fit well into
> one situation does not mean that it cannot be adapted to better fit
> into another framework.
True, but I fail to see how adding a vftable to it is adapting it to better
fit into AbiWord.
> Subclassing with virtual functions is one such way of achieving
> this functionality.
Yes, "one" way. Why not select a "better" way (smaller and faster) when
given the option?
> I'm sorry if C++ doesn't give you virtual functions
> for free (or by default) like Java does.
You're sorry "if"? Shouldn't that have been your sorry "that"? Btw, I'd
_really_ like to see you trying to explain that "every function is virtual"
in Java comes "for free". ;->
> They don't add all that much bloat either, though.
So a little bloat is OK? I'm sorry, but I think _that's_ flawed thinking.
> However, overloaded freestanding functions are a major flaw in C++, and
> desperately show the problems that arise by not having some common
> anscestor class and reek of C++'s C-based heritage.
Perhaps you should try that argument in e.g. comp.lang.std.c++. Expect to be
educated. :-)
> XAP_AbiObject strives to be this anscestor class.
>
> Consider this scenario:
>
> my way:
> UT_Map::add (Object key, Object o) {
> UT_uint32 k = key.hashcode();
> m_pimpl->add (o, k);
> }
>
> your way:
> UT_Map::add (std::string key, Object o)
> UT_Map::add (const void * key, Object o)
> UT_Map::add (const char * key, Object o)
> UT_Map::add (Object key, Object o)
> UT_Map::add (int key, Object o)
> ...
No no no no no no no! Absolutely NOT!
UT_HashableString
{
public:
UT_HashableString(const UT_String s /* value semantics */);
UT_uint32 hashcode() const;
...
private:
UT_String m_str;
};
UT_Map::add(UT_HashableString key, Object o)
{
UT_uint32 k = key.hashcode();
m_pimpl->add (o, k);
}
or
UT_uint32 hashcode(const UT_String& s);
UT_Map::add(const UT_String& key, Object o)
{
UT_uint32 k = hashcode(key);
m_pimpl->add (o, k);
}
I sure hope you're not planning on allowing windows, views and documents
(all planned to be inherited from the XAP_AbiObject?) to be keys into the
same map, are you?!
> Is my solution easily extensible and flexible, not to mention clean?
Not. To put icing on the cake it's not (type-) safe either.
> Is the alternative?
I hope I have displayed one of the alternatives.
> vftables and interfaces aren't bad things in and of themselves.
> Interfaces are the best thing about an OO language - hands down.
It's at least one of the good things, but your still locked into the
thinking that virtual functions is the only way to express interfaces -
possibly because that's the only way to do it in Java. C++ is a much more
powerful language, giving you other ways to express yourself.
> I fail to see how hashcode() violates your "value semantics" argument.
Me too, especially since I never used that as an argument.
> ref() and unref() might, but only if you convince me of the below point:
First let me explain that these are two separate issues. You're trying to
cram two unrelated behaviors into one class. That is a design error.
Judging by the function names, are you planning on creating some kind of
*manual* ref-counting scheme? You should know by now this doesn't work, we
have been down that road before - I can't remember which class it was, but
one d'tor didn't unref() as it should and then we got mem-leaks.
I would also like to point out some obvious problems with/errors in the
xap_AbiObject class:
1. Isn't that really a utility (UT_) class?
2. The way it's designed it makes it impossible to reference const objects.
Perhaps ref(), unref() and sink() should be const, and m_refs should be
mutable?
3. The experimental memory management only contains "delete" for objects
partially constructed (when exception is thrown). Perhaps the normal
"delete" also should be added. What about the array versions, are they going
to go through the system memory management?
4. It contains the Java construction "equals". Silly, and dangerous, since
in C++ we have a global operator==() for any types which does comparison.
Since this (virtual) compare function returns equality of pointers, no
subclass can change that behaviour without violating the LSP (Barbara
Liskov - "Liskov Substitution Principle"). It also allows comparison of any
types inherited from it, e.g. a string and a document. Not only silly, but
completely wierd and useless. If it is to only let objects compare for
"value" equality (not pointer equality) it should be a pure
virtual. To at least try to mimic C++ it should also be named "equal"
without the "s".
5. It contains no copy constructor, making it completely useless unless we
add a clone() member function. Note that such a clone() function can't use
a covariant return type since (bloody) M$ can't get their act together (this
is proven). How can you ref-count anything unless you can make copies of
it???
> > If the strings are to be ref-counted, inheriting XAP_AbiObject was
> > precisely the wrong way to do it. It should be a property of the
> > string class, where it stores a ref-count in the string buffer.
>
> I fail to see the difference, but maybe that's just because I haven't
> given it much thought.
Large changes like these need some thought.
> Please convince me that my way is wrong and yours is right,
> because I'm a skeptic. All I see is a different place where the integer
> ref-count is stored and no bad side effects arising from that.
If you still don't trust me after this essay, ask again and I will explain
it.
Perhaps the following links can provide some interesting reading also?
http://www.gotw.ca/gotw/043.htm
http://www.gotw.ca/gotw/044.htm
http://www.gotw.ca/gotw/045.htm
/Mike
This archive was generated by hypermail 2b25 : Sat Jun 02 2001 - 21:48:33 CDT