Re: ANN: AutoNewPtr (oh yes, yet another smartpointer)
* Daniel Kr?gler:
On 11 Mai, 17:42, "Alf P. Steinbach" <al...@start.no> wrote:
I just coded this up, and hopefully by mentioning it here I'll get some
useful
feedback, like, bugs, shortcomings, improvements...
Let me begin with some apologies that I only took
a short sweep over the code, so that I have not
*fully* grasped all it's miracles.
Heh heh. There are no miracles. :-)
Therefore it would
be a shame if I would start with a lot of praises - on
the other side the same reason hinders me to unpack my
large whip, so all in all this seems like some kind of
balance ;-)
So here are some points that caught my eyss:
1) AutoNewPtr<>::operator Base(): Is practically useless, see
[class.conv.fct]/1:
"[..] A conversion function is never used to convert a (possibly cv-
qualified)
object to the (possibly cv-qualified) same object type (or a reference
to it),
to a (possibly cv-qualified) base class of that type (or a reference
to
it),[..]"
Thanks, I didn't remember. Anyway it was just a convenience wrapper
for the function on the line above (managingPtr()), which is also just
a convenience function. "was": that operator now removed. :-)
2) CheckingPtr: CheckingPtr::CheckingPtr( T* p ) is not exception
safe, if I
read your code correctly. Consider that
new detail::AutoClearingRefCountedDeleter<T>( p )) fails, and no-one
cleans up
p. Note that shared_ptr, which has basically to cope with the same
problem,
does solve this, by carefully ensuring that p will be cleaned-up even
under
those circumstances.
Thanks again, that's a bug.
Fixed.
I think it's an interesting problem how to do that without any space
overhead for a compiler that doesn't support function try-blocks. I
guess one solution would be to use a special smart pointer with
implicit constructor as formal argument. Then release() in body.
3) Personally I don't like the choice to use the notion of "void" to
signal a
meaning of "empty" (makeVoid, isVoid, ...). I assume that you have
chosen this
different name to prevent the meaning "dubiety" of empty in
shared_ptr? ;-)
Nope, "void" is just because "null" is so overused, and because Eiffel
has this beautiful concept of nullpointer-ness not as a value (which
is value that logically doesn't fit into type system) but as /state/.
Whatever the reason was I strongly propose to clarify it's meaning.
Yes. An AutoNewPtr or CheckingPtr can be in either void or valid
state. If it is void it doesn't refer to any object, and -> throws.
4) DestructionEventSource:
a) The decision to introduce the pair
void setDestructionEventHandler( IHandler& handler )
void clearDestructionEventHandler()
Since there obviously exists an "empty" state (otherwise clearing
does not make much sense), I probably would have reduced this pair
to the single function
void setDestructionEventHandler( IHandler* handler )
This decision is largely a matter of taste, but you were asking also
for "wild ideas" and this is one of mine;-)
Yes. It's just internal implementation functionality, but anyway I
like to signal, in the first case, that it will not be nullpointer,
and in the second case, that it is. E.g. makes it easier to debug
(you can just set a breakpoint on non-null or null, as you please).
b) What was the main reason to provide copy constructor and copy
assignment of DestructionEventSource (with an unusual semantic) at
all?
As far as I see, the corresponding user classes are non-copyable.
The user classes can be anything. They are the classes for the
pointee objects, deriving from DestructionEventSource (for AutoNewPtr
that derivation is added automatically if it isn't there in the client
code class). So the user classes can be copyable, yes.
And if such a referred object is copied, and the copy is destroyed,
then one does not want that to set the smartpointer(s) for the
original object to void. :-)
A similar problem and similar "unusual" copy semantics solution is in
Boost signals.
5) Second overload of throwX( char const s[] ): I wonder concerning
the
unusual argument type, which is basically the same as const char*. Is
this supposed to be a documentation to the reader?
Yes.
If you want a
proper
protection against potential NULL arguments I propose to use a
template
based approach like this one:
template<std::size_t N>
FUNCATTR_NORETURN inline bool throwX( char const (&s)[N] ) {
... // Your code
}
This will only accept char arrays.
Yes, but given that the first overload is needed, this safer one,
which would be safer if it was alone, is not providing extra safety.
The char const[] overload is there because MSVC 7.1 didn't like it
much (as I recall I got an ICE, Internal Compiler Error) when I just
had the std::string argument overload. Checking... Dang, can't
reproduce, but it was there.
Anyway, this very basic functionality is what I keep coming back to.
Earlier I used to elaborate greatly on that throwing. E.g.
functionality for picking up throwing context, logging, operators and
macros for e.g. "<<" for this and that error data type, etc. ad
nauseam. But it gets too complicated and non-portable. However, I
guess there should be some customization point in there. Not sure
what would be a good solution for that.
PS: Yes I'm aware that adding things to namespace std is formally
prohibited,
except specializations of existing templates. The stuff added is such
things
that will be added by C++0x. I think it's better to just use namespace std
than
to keep changing code as things migrate boost -> tr1 -> std.
I probably would have used the boost functionality
explicitly since you use it already where the standard
has no similar feature (Not even for C++0x).
Well, the idea is to write std::xxx for whatever is or will be std::.
So that if the code only uses std:: things, then with a C++0x
compiler or compiler with those C++0x features (although I don't (at
least yet) support that level of granularity), it won't need the Boost
library.
Thanks for the feedback & suggestions. :-)
Cheers,
- Alf
--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]