[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