[Development] Common base class for all socket types

Corentin Jabot corentin.jabot at gmail.com
Wed Nov 7 00:53:57 CET 2012


2012/11/7 Thiago Macieira <thiago.macieira at intel.com>:
> On quarta-feira, 7 de novembro de 2012 00.07.41, Corentin Jabot wrote:
>> The following seems like a good subset of the QAbstractSocket
>> interfaces that also makes sense for QLocalSocket
>
> I don't want you to list functions that are common. I can do that by myself.
>
> I want you to explain *why* you need those. And while you're at it, let us
> know whether they should be shared with QProcess and QNetworkReply too.
>
>>
>> void abort ()
>> void disconnectFromHost ()
>
> No for disconnectFromHost so long as connectToHost() isn't common. You'll have
> to make do with close().

Makes sense

>> SocketError error () const
>
> I'd have liked to replace this with a QtNetwork-wide error enum, the one in
> QNetworkReply.

Would be neat !

>> bool flush ()
>> bool isValid () const
>
> Explain why you need isValid and why isOpen or others don't suffice.

Well if that method is useful in both QLocalSocket & QAbstractSocket,
i guess it should be common.
But if isOpen() do the trick, then... maybe that method is not useful
in the first place,
should it be deprecated. or the doc should state "see also isOpen()" ?

>
>> qint64 readBufferSize () const
>> void setReadBufferSize ( qint64 size )
>
> I completely agree with those two. They should be in QIODevice itself.

Should flush() be in QIODevice too ? I think there could be other
use-case for it

>
>> bool setSocketDescriptor ( int socketDescriptor, SocketState
>> socketState = ConnectedState, OpenMode openMode = ReadWrite )
>
> No on this one. I don't think that replacing an existing socket makes sense in
> the common base. You usually need to know that the socket type is compatible
> with the class that you're using. Therefore, this does not need to be common.

Agree, It depends indeed to much of the implementation

>
>> int socketDescriptor () const
>
> Yes on this one.

You said disconnect should not be common if connect is not, and, I
tend to agree.
Isn't that the same for socketDescriptor/setSocketDescriptor ?

>
>> SocketType socketType () const
>
> Yes and QLocalSocket needs to differentiate a local socket and an abstract
> local socket.
>
>> SocketState state () const
>
> Makes sense.
>
>> bool waitForConnected ( int msecs = 30000 )
>> bool waitForDisconnected ( int msecs = 30000 )
>
> No on both of those: connecting is specific on the type, therefore
> waitForConnected is specific too. If disconnectFromHost isn't common either,
> then waitForDisconnected shouldn't be either.
>
>> The SocketError & SocketState enums have already similar values in both
>> class
>>
>> SocketType should be in TcpSocket, UdpSocket, LocalSocket - ( could
>> also be NamedPipeSocket ? )
>
> It should differentiate based on the implementation in the backend. There's
> also the NonceTcpSocket possibility for QLocalSocket. I agree we need it.
>
>> The following signals could also be common (again, it's about using
>> the same enums):
>> void connected ()
>> void disconnected ()
>> void error ( QAbstractSocket::SocketError socketError )
>> void stateChanged ( QAbstractSocket::SocketState socketState )
>>
>> I dont see how the connectTo* methods could be shared, and I don't
>> think it would be a good idea to try to do so.
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
>
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>



More information about the Development mailing list