[phobos] Breaking changes for std.socket improvement
Masahiro Nakagawa
repeatedly at gmail.com
Wed Sep 8 07:02:37 PDT 2010
Thanks for the review.
On Sun, 05 Sep 2010 15:29:36 +0900, Andrei Alexandrescu
<andrei at erdani.com> wrote:
> I already wrote the following code review for
> http://bitbucket.org/repeatedly/scrap/src/tip/socket.d before seeing
> Masahiro's new message that we should scrap that review. I'm sending
> this in case it applies to his new work based on Asio.
>
> * Line 67: don't use typedef - it will go away
OK.
> * 87: feel free to insert a static assert(sockaddr_storage.sizeof ==
> ...) to make sure the compiler did as you expected.
I think static assert doesn't need because the detail of sockaddr_storage
is not important.
I will add sockaddr_storage to std.c.windows.
> * 270: enforce(val, new SocketException(...))
I forgot to rewrite.
> * 304-309: lowercase? We have visibility protection because of
> AddressFamily.
OK.
> * 362: Why is Protocol a class? It has all public members and no
> overridable methods. (Also, constructors and public members often are
> questionable.)
>
> * 433: Same question about Service.
>
> * 539: And same question about InternetHost.
>
> * 546: why not a ref hostent?
>
> * 553: no need for the .idup
I don't know the detail. I will remove those classes in new module.
> * 663: I'm weary about every module adding its own exception type, but
> this is subject to a separate investigation we should do for all of
> Phobos.
I agree. I think Phobos guide should describe the exception design.
> * 694-698: Public members of a class mean that I can change any of them
> to random values without breaking the consistency of the object. Is that
> true for AddressInfo?
I think yes for AddressInfo bacause AddressInfo is a container for
getaddrinfo.
I will change AddressInfo to struct.
> * 752: ref addrinfo
OK.
> * 777 and others: which of these methods do you think should be
> overridable? Probably a small subset if any. Then they should be final.
I mentioned above.
> * 947: Is there something that naturally differentiates the two types of
> addresses? If so, you could get rid of Type.
It's a matter of taste.
In general, User uses AddressInfo for Socket.
So, IPv4Address and IPv6Address are almost invisible.
I think IPAddress only is not handy if User uses IPv4 or IPv6
representation directly.
I will add some functions to IPv4 and IPv6 in new module.
> * 1294: I'm not an IPv6 pro, but looks like a little hierarchy would
> work here (with e.g. IPv4 and IPv6 inheriting a common base). Maybe
> LocalAddress could be snuck in too.
What is a common base?
IP Address contains IPv4 and IPv6, but IPv4 and IPv6 doesn't inherit IP
Address.
In addition, IPv6 doesn't have the compatibility to IPv4.
> * 2310: what does byes mean?
I don't know, but I think byes stands for "blocking yes".
> * 2520, 2541 etc.: Return size_t.
Oops.
> * 2934: select() has well-known issues. Any plans to support the newer
> system-dependent APIs or libevent?
I will remove select() from Socket in new module.
I am thinking about std.event.
This module supports system-dependent APIs(kqueue, epoll, etc...) and
abstraction layer.
Masahiro
More information about the phobos
mailing list