Review for gkiagia's krfb port to StreamTubeServer

[Olli Salli]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 27.09.2011 17:19, Olli Salli wrote:
> On 25.09.2011 23:35, George Kiagiadakis wrote:
> 
>> Maybe a small struct would make it more clear, yes. I don't much like
>> pair return values, since it is not very clear what you are doing if
>> you use one of its members directly, for example:
> 
>>    setContact(sts->tcpConnections()[qMakePair(QHostAddress::LocalHost,
>> port)].second);
> 
>> is not as clear as:
> 
>>    setContact(sts->tcpConnections()[qMakePair(QHostAddress::LocalHost,
>> port)].contact);
> 
>> and I would have to write something like this to make it more clear:
> 
>>    ContactPtr contact =
>> tcpConnections()[qMakePair(QHostAddress::LocalHost, port)].second;
>>    setContact(contact);
> 
> 
> Right, thanks for the feedback there. I'll derive a trivial struct
> Tp::StreamTubeServer::RemoteConnectionContact from QPair<AccountPtr,
> ContactPtr> with accessors account() and contact() returning the
> corresponding QPair members (but more readable). Then it's the best of
> both worlds, don't you think? I'll add a placeholder pimpl for the
> struct, so we can later add more info for the contacts if that's
> available from DBus.

Ok, this became STS::RemoteContact. There's now also {STS,STC}::Tube
which on their part make the types of the connection hashes cleaner, and
are also used in the tubes() accessors for similar gains to what
RemoteContact achieves in the connections hash.

> 
> Do you think we need a similar struct for the source address pair (key
> of these mappings) too? IMO, that wouldn't make code looking up stuff
> from these mappings any cleaner though, as you can already see from an
> address and port being passed what those pair members are, so I guess
> this is more beneficial for the mapping value types.
> 

Actually we even couldn't do this consistently, because we already use
QPair<QHostAddress, quint16> in return types in many places in released
(Outgoing)StreamTubeChannel APIs, and PendingStreamTubeConnection. To be
consistent, we'd need to change those return types too, but we can't
because we need to preserve ABI compat there.

Also, it'd be weird to define a type for a pair of host address + port,
as that's in no way a Telepathy specific concept. I rather think
QtNetwork should include one, as a host address and a port are seldom
useful alone - this type could be something like QDateTime, which
combines a QDate with a QTime.

Br,
Olli
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJOgiCZAAoJEAQQkupGanj4pDIIAMTdripi0Rb8yx+3WOczjBll
1SXnbM+iJOKe9Isc+vdLcHEv1hu7TJlqgnrV3rNnSnszaIwbpA7fR835ZrL6Zv4f
6ItoqlQWcGHE08obYN/Wq8mNzDEEB2X1rd16itZ+ZRC5IdSw1zNodspP4S3kueis
CDa8dMXkrR4onP8vsuVAPmuEylodXYw4ja1NMpVZs7V72TCbmvvxAhSmr6KBoQ++
H/1TLpZdyb9U6NcK1J+T2V82aIsnLLrmlzJ5xdYuSx73h/S5B9Ormr4JaZV3WKCn
DLChMGlv1gc2diPIt06RL/ELC2UGKS52vO8BuO2EdTYYw0RJtaGZg619Dke9H8M=
=j1Xq
-----END PGP SIGNATURE-----