Sorry for the very long delay in the reply... On 29/11/12 10:02, George Kiagiadakis wrote: > On Thu, Nov 29, 2012 at 12:03 AM, David Edmundson > <[email protected]> wrote: >> However, it's quite a large change in direction I think, so I want >> input from you guys. > > +1 from me, I would say.
+1 from me too, it looks like a good idea. >> - None of the methods in Tp::Contact are virtual, which means if we >> were to override the block method to show the "are you sure" dialog >> first (for example), you would get a different behaviour happen >> depending on whether you had casted or not. > > That's the only real problem that I can see. We should be extremely > careful to always cast objects to what they really are before using > them. Do we really need to override methods? If we do, which ones? Can we have those fixed in the next TpQt ABI break? >> - Any code which tried to use KTp::Contact with a standard >> Tp::ContactFactory would crash. > > Not very difficult to fix... Usually we do not get a KTp::Contact directly but Tp::ContactPtr instead, so you are sure that you cannot call KTp::Contact methods from that. We should have a typedef Tp::SharedPtr<KTp::Contact> KTp::ContactPtr so that if you cast a Tp::ContactPtr to a KTp::ContactPtr and check if it isNull() (and if you don't do it after a cast, then you are probably crazy), you are sure that you can use it. So I don't see a big problem here. Just one more idea: in several places we need the Tp::Account together with the Tp::Contact, (for example all the "actions" require both, and the contact grid widget requires 2 methods to get the contact and the account). Does it make sense to have a method Tp::Account KTp::Contact::account() const so that we can pass just the KTp::Contact everywhere and always be able to access to the account? (Actually if I understand it correctly the GlobalContactManager should allow that too, but I didn't have much time to look at it yet, sorry) Cheers, Daniele _______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
