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
* 87: feel free to insert a static assert(sockaddr_storage.sizeof ==
...) to make sure the compiler did as you expected.
* 270: enforce(val, new SocketException(...))
* 304-309: lowercase? We have visibility protection because of
AddressFamily.
* 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
* 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.
* 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?
* 752: ref addrinfo
* 777 and others: which of these methods do you think should be
overridable? Probably a small subset if any. Then they should be final.
* 947: Is there something that naturally differentiates the two types of
addresses? If so, you could get rid of Type.
* 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.
* 2310: what does byes mean?
* 2520, 2541 etc.: Return size_t.
* 2934: select() has well-known issues. Any plans to support the newer
system-dependent APIs or libevent?
Andrei
On 07/05/2010 02:08 PM, Masahiro Nakagawa wrote:
std.socket lacks some features and I don't like current design. So, I
rewrote this module.
Summary:
- Change API of Protocol, Service, and InternetHost.
-----
/* old */
Protocol proto = new Protocol; // construct before check
enforce(proto.getProtocolByType(ProtocolType.TCP)); // check
// do stuff
/* new */
Protocol proto = enforce(Protocol.getByType(ProtocolType.TCP)); //
construct after check
// do stuff
-----
I leaves these classes for aliases(getaddrinfo can't get aliases).
- Add AddressInfo class(wraps C's addrinfo struct)
AddressInfo resolves some address familes. I added AF_UNIX support.
This class is useful for Socket and Endpoint initialization.
In the future, above classes should be moved to std.net(or std.net.dns).
- Endpoint
I think Address should not have port. Address is a address, not a endpoint.
So I added Endpoint structs(derived their name from Asio) and Endpoints
use corresponding Addresses.
-- IPEndpoint
This struct supports IPv4 and IPv6 using IPAddress.
-- LocalEndpoint
This struct supports local path using LocalAddress.
- Socket
Socket is a template bacause Socket should support any address familes.
Old Socket uses class-inheritance, but newFamilyObject is
bad(newFamilyObject can't use user-defined class).
Some methods of old Socket used return-value checking :(
- SocketSet
add and remove methods are typesafe-variadic function.
- Add const and attribute
Sources:
- socket.d
http://bitbucket.org/repeatedly/scrap/src/tip/socket.d
unittest is a simple example.
- socketstream.d
http://bitbucket.org/repeatedly/scrap/src/tip/socketstream.d
std.stream will be replaced or eliminated. This module need?
- LocalEndpoint sample
http://bitbucket.org/repeatedly/scrap/src/tip/sample/echo_server.d
http://bitbucket.org/repeatedly/scrap/src/tip/sample/echo_client.d
Contribution from satoru_h
I would like to commit this change. What do you think?
Masahiro
_______________________________________________
phobos mailing list
[email protected]
http://lists.puremagic.com/mailman/listinfo/phobos
_______________________________________________
phobos mailing list
[email protected]
http://lists.puremagic.com/mailman/listinfo/phobos
