[boost] Re: Sockets!

From: Jessie Hernandez (jessie_at_[hidden])
Date: 2003-10-25 01:43:39

Thorsten Ottosen wrote:

> 
> 
> "Jessie Hernandez" <jessie_at_[hidden]> wrote in message
> news:bnabpu$vgt$1_at_sea.gmane.org...
> [snip]
>> I'd like to get feedback on the current state of the library and see what
>> can be improved (I'm mostly interested in the design aspect).
> 
> I'm really looking forward to a socket library in boost so I appreciate
> all work done in that regard.
> 
> Just some quick comments to your example (its hard giving detailed
> feedback on the desing when there is no design document):
> 

Yes, I hope to provide one shortly.

> 
> domain::address_type addr( host<domain>( "www.boost.org" ).address().ip(),
> 80 );
> 
> why is it necesary to say host.address().ip() ? why can't a host have an
> ip directly?

This is because the concept of an IP address only applies to IPv4/IPv6
addresses. An address in the local (or Unix) domain is just a file path,
for example.

Originally, I had the host class only apply to the IPv4/IPv6 domains, but
then I thought the host concept can be applied (or at least "faked") to
other domains.

What I will do, though, to make the host class easier to use, is specialize
it for ipv4/ipv6 and add the ip member function (which will just be a
wrapper for "address().ip"). Thanks for pointing this out.

> why doesn't the host's template parameter default to ipv4?
> I think I might like two typedefs instead: ipv4_host and ipv6_host
> 

Yes, this is what I envisioned too, I just haven't had time to write
ipv6_host ;-).

> sockstream.close();
> 
> I would prefer this was put in the destructor for exception safety reasons
> and ease of use.
> 

It is already in the destructor. The only reason I provided it in the tests
was to demonstrate "good usage". It's the same as the fstream class, where
if you don't call close(), it will be called in the destructor anyways.

> I have no idea about what the follownig does, the names don't seem very
> intuitive:
> 
> hostent* boostEnt = ::gethostbyname( "www.boost.org" );
> 

These are not part of the library. This is in the "test_host" function
simply to make sure that the host class is returning the same data as the
system functions.

> Why all this nesting:
> 
>  sockstream.rdsocket()->blocking( false );
> 
> ? I would prefer sockstream.enable_blocking()/disable_blocking() [naming
> not important]
> 

This "nesting" also exists in the fstream classes, though it's "hidden"
because the member functions are "duplicated" in the stream class (e.g.,
when you call fstream::is_open(), it's actually calling
fstream::rdbuf()->is_open()). I will duplicate these functions at the
stream class level for convenience, though.

> Why does rdsocket() return a pointer?
> 

Same reason why fstream::rdbuf() returns a pointer. It was done for
consistency with the fstream classes.

> Naming in general; prefer read_socket() to rdsocket(); prefer protocol()
> to proto() prefer is_connected() to connected(), set_port() to port()
> 

1) rdsocket was named to resemble rdbuf in the fstream classes (this can be
changed, though, if many would like).
2) protocol() cannot be renamed because there is a class named protocol.
3) is_connected, etc. was chosen also to resemble the functions in the
basic_ios class (they are named good(), eof(), fail(), and bad()).
4) port() was chosen, again, for consistency with the basic_ios class. For
example, basic_ios::fill can be used to either return the pad character or
to set it. Same idea with port().

> don't put exception specifications on functions.
> 

Yes, this is on my TODO list.

> Overall I think the usage seems simple.
> 

Thanks for taking the time to look at it!

--
Jessie Hernandez