Aleksey Kontsevich wrote:
>> I'm not sure I understand what you mean here... Maybe you could show a
>> very brief example of the structure of your code?
>
>> trigger() { m_Name=""; };
>> setName(string &N) {m_Name=N;};
>> setConn(connection &C) {
> > m_Conn.RemoveTrigger(this);
> > m_Conn=C;
> > m_Conn.AddTrigger(this);
>> };
> you can add also somethin like this (If user wants to swith off the trigger
> temporarily):
> switch_off() {m_Conn.RemoveTrigger(this);};
> switch_on() {m_Conn.AddTrigger(this);};
>
> For example I making such class
>
> TMyApplication : pqxx::trigger
>
> and I want my own constructor, something like this:
>
> TMyApplication(char ini_file) {
> // getting Conn string and trigger name
> ...
> // making some memory allocations
> ...
> // activate the trigger
> setName(N);
> setConn(C);
> };
> ~TMyApplication()
> {
> // clear memory
> ...
> }
>
>> Initializing objects with multiple method invocations like this introduces
>> opportunities for bugs: what if you accidentally call setName() on an
>> object that has already been setConn()'ed? What if you don't set a name?
>> What if you call setName() more than once before the setConn()? What if
>> you forget to call setConn()? What if you mistakenly passed a connection
>> and name to the constructur *and* called setName() and/or setConn()
>> afterwards?
>
> I don't see any problem here: old code will continue to work as it is and
> new
> one will get some new opportunities. I looked through you code: there is no
> problem if m_Name is not set or differs - get_notifs() method just will not
> call the trigger::operator()(int) - trigger is switched off. The same thing
> on m_Conn.
Hi Aleksey,
First of all, it's not a question of backward compatibility -- it's a
question of introducing lots of different ways that a class can be used,
introducing gazillions of combinations and possibilities for bugs. Think
about it, any trigger can suddenly be in any of 4 states:
* no connection and no name set
* connection set, no name set
* no connection set, name set
* connection and name both set
Every method of the trigger must check the state of the trigger and must
do different things depending on this state. This not only makes things
slower, but also make them more error-prone. Your example makes it seem
very easy, but it is oversimplified: for instance, it only works if you
set the name _first_, and then the connection. The following
implementation would be needed to support any calling order:
trigger(connection &C, string const &name)
: m_Conn(&c),
m_Name(name)
{
}
trigger()
: m_Conn(0)
{
}
setName(string const &name)
{
if (m_Conn != 0 && m_Name != "")
m_Conn->RemoveTrigger(this);
m_Name = N;
if (m_Conn != 0 && m_Name != "")
m_Conn->AddTrigger(this);
}
setConn(connection *C)
{
if (m_Conn != 0 && m_Name != "")
m_Conn->RemoveTrigger(this);
m_Conn = C;
if (m_Conn != 0 && m_Name != "")
m_Conn->AddTrigger(this);
};
~trigger()
{
if (m_Conn != 0 && m_Name != "")
m_Conn->RemoveTrigger(this);
}
But this adds tremendous complexity to every method, not to mention that
this complexity extends to every addition that is made to the class in
the future!
(Sidenote to Jeroen: why doesn't trigger have a copy constructor and
assignment operator? This is a bug, right?)
As Jeroen says in his reply (which I only just received), initializing
the connection in the derived class is probably not something you'd want
to do: if the connection and the trigger are both technically owned by
the subclass, then there should be an ownership relationship, not an
inheritance relationship. Doing it any other way would probably just get
you into a lot of trouble -- exception-safety-wise,
initialization-order-wise, etcetera. Trigger classes should really only
be derived from to trigger actions in preexisting, different objects --
triggers are not the actions, they are simply a glue layer intended to
allow the libpqxx user to trigger the actions.
I hope this is enough to clarify why this isn't a good idea. :-)
Cheers,
Bart
_______________________________________________
Libpqxx-general mailing list
[email protected]
http://gborg.postgresql.org/mailman/listinfo/libpqxx-general
