Hi,
I am working on the second pass of this feature and write here because of
the legitimate concern regarding new classes to add.
PR here https://github.com/php/php-src/pull/17926
I agree also on adding a namespace on top (Sockets ?) and changing the
class names.
Note that after that I plan at least one more PR (datagram support, reusing
ethernet class as usable one, ...).
Kind regards.
Hi,
I am working on the second pass of this feature and write here because of
the legitimate concern regarding new classes to add.
For those that want to quickly see the actually classes, here is a stub
link:
https://github.com/php/php-src/pull/17926/files#diff-190e64a9475f39f2b575e6032d6539b504b33fe534965e5bb3512fe430fe1846
I agree also on adding a namespace on top (Sockets ?) and changing the
class names.
I think it would be good to note that there are already two classes defined
in socket extension:
final class Socket
{
}
final class AddressInfo
{
}
So it might look a bit strange if those two are not namespaced and the new
ones are. The AddressInfo is a bit unfortunate name though. The namespace
seems cleaner but not sure if prefix would be more consistent. Although
having SocketPacket is not exactly nice so probably Socket\Packet is
better. But I don't really mind. I would just really prefer to not
introduce those classes without namespace or prefix.
Regards
Jakub
Hi,
I am working on the second pass of this feature and write here because of
the legitimate concern regarding new classes to add.For those that want to quickly see the actually classes, here is a stub
link:
https://github.com/php/php-src/pull/17926/files#diff-190e64a9475f39f2b575e6032d6539b504b33fe534965e5bb3512fe430fe1846I agree also on adding a namespace on top (Sockets ?) and changing the
class names.I think it would be good to note that there are already two classes
defined in socket extension:final class Socket
{
}final class AddressInfo
{
}So it might look a bit strange if those two are not namespaced and the new
ones are. The AddressInfo is a bit unfortunate name though. The namespace
seems cleaner but not sure if prefix would be more consistent. Although
having SocketPacket is not exactly nice so probably Socket\Packet is
better. But I don't really mind. I would just really prefer to not
introduce those classes without namespace or prefix.
ACK Sure fair point ; in practice prefixing seems the least disruptive
change.
Regards
Jakub
Hi,
I am working on the second pass of this feature and write here because of the legitimate concern regarding new classes to add.
For those that want to quickly see the actually classes, here is a stub link: https://github.com/php/php-src/pull/17926/files#diff-190e64a9475f39f2b575e6032d6539b504b33fe534965e5bb3512fe430fe1846
I agree also on adding a namespace on top (Sockets ?) and changing the class names.
I think it would be good to note that there are already two classes defined in socket extension:
final class Socket
{
}final class AddressInfo
{}
So it might look a bit strange if those two are not namespaced and the new ones are. The AddressInfo is a bit unfortunate name though. The namespace seems cleaner but not sure if prefix would be more consistent. Although having SocketPacket is not exactly nice so probably Socket\Packet is better. But I don't really mind. I would just really prefer to not introduce those classes without namespace or prefix.
Those two classes predate the Namespaces in bundled PHP extensions [1] RFC, I do think adding a namespace is better.
And we could also move the two classes into the new namespace and add class aliases for the global ones, considering they are quite recent additions to PHP.
Best regards,
Gina P. Banyard
[1] https://wiki.php.net/rfc/namespaces_in_bundled_extensions
Hi
Those two classes predate the Namespaces in bundled PHP extensions [1] RFC, I do think adding a namespace is better.
And we could also move the two classes into the new namespace and add class aliases for the global ones, considering they are quite recent additions to PHP.
I agree that adding a namespace would be appropriate here. Looking at
the sockets.stub.php, it pollutes the global namespace quite a bit. I
probably miscounted, but it appears to be at least 10 different prefixes
for the global constants.
It would probably be in order to also alias all the constants into the
namespace (possibly converting them to enums where appropriate) and then
in a follow-up version deprecate the global ones. I think that would
bring quite a bit of positive impact, for minimal effort.
As for the classes themselves: It does not appear to be defined in our
naming policy [1], but the properties should likely use camel-case, as
that is the established convention for userland code. And I'm also
curious why the port properties are strings, not ints? And what is the
payload object, is that just stdClass?
Best regards
Tim Düsterhus
[1]
https://github.com/php/policies/blob/main/coding-standards-and-naming.rst
Hi
Those two classes predate the Namespaces in bundled PHP extensions [1]
RFC, I do think adding a namespace is better.
And we could also move the two classes into the new namespace and add
class aliases for the global ones, considering they are quite recent
additions to PHP.I agree that adding a namespace would be appropriate here. Looking at
the sockets.stub.php, it pollutes the global namespace quite a bit. I
probably miscounted, but it appears to be at least 10 different prefixes
for the global constants.
Ok I ll go the namespace route then, fair point.
It would probably be in order to also alias all the constants into the
namespace (possibly converting them to enums where appropriate) and then
in a follow-up version deprecate the global ones. I think that would
bring quite a bit of positive impact, for minimal effort.
As for the classes themselves: It does not appear to be defined in our
naming policy [1], but the properties should likely use camel-case, as
that is the established convention for userland code. And I'm also
curious why the port properties are strings, not ints?
Yes it is a definition mistake it means to be a int indeed.
And what is the
payload object, is that just stdClass?
Not really it's supposed to be one of the new classes.
Best regards
Tim Düsterhus[1]
https://github.com/php/policies/blob/main/coding-standards-and-naming.rst
Hi
Am 2025-03-05 13:11, schrieb David CARLIER:
And what is the
payload object, is that just stdClass?Not really it's supposed to be one of the new classes.
Okay, please check if you can find a more appropriate type in the stub
then. ?object is awfully generic (and thus useless to the user).
Best regards
Tim Düsterhus