From: Eivind Eklund <eivind@...>
Date: 2004-04-02T21:52:46+09:00
Subject: Re: Proposed patch to add SSL support to net/pop.rb

(NOTE: The below is just an opinion piece, and I trust you to make a
good call on this.)

On Fri, Apr 02, 2004 at 07:55:01PM +0900, Minero Aoki wrote:
>   In mail "Re: Proposed patch to add SSL support to net/pop.rb"
>     Eivind Eklund <eivind@FreeBSD.org> wrote:
> 
> > > I think that #start has taken too many parameters.
> > 
> > Apropos: Nobody has mentioned the use of named parameters yet.  That
> > would give something like the following (for 1.8 - in 2.0, we have extra
> > syntactic sugar):
> 
> 1.  Matz has already discouraged use of ":name => value"
> style arguments.  So you must explicitly write it as:
> 
>   Net::POP3.start(...., {:certs => c, :verify => v})

That had passed me by.  The change makes the calling convention less
beautiful :-(

> Note that "a: value, b: value" syntax is not portable.
> Ruby 1.8 has not implemented it.

I'm aware of that.  I like the new syntax, but has used :name => value
for it so far (I'm a big fan of named parameter for optional parameters
- my experience is that they can often make designs much nicer.)

> 2.  If you use a hash argument for :certs and :verify,
> you also want to pass other arguments in the same way e.g.
> 
>   Net::POP3.start({:host => 'mail.example.com', :port => 110,
>                    :user => 'aamine', :password => 'abcdefgh',
>                    :apop => true,
>                    :ssl => true, :certs => c, :verify => v})

Are the other parameters optional? (Port and password look like they
could be, but aren't host and user always required?)

Personally, I usually prefer to use positional parameters for required
arguments and named parameters for optional parameters.  I've found this
to be particularly useful for various forms of factories, as these often
take a bunch orthogonal parameters where only some need to be specified.

> But it is too late.  The only way is checking the class of
> the first argument, that is a bad design.

I wouldn't be quite so categorical, but if some of the existing
parameters also should be made optional, I'd probably introduce a new
method for it.  Net::POP3.connect, perhaps, if that isn't already taken?
(Though I'll admit to liking "start" better :-(

> 3.  Keyword arguments should not be used to hide the bad
> design (too many parameters).

I'll challenge the idea that many parameters *in itself* is bad design.
Many positional parameters is almost always bad design.  However, many
optional parameters can be an indication of bad design - but it can also 
be an aspect of good design.  For the case in question, it looks to me
like it would be a convenient API for somebody that was going to write
something that just used a specific POP connection - ie, for standard
"throwaway scripts", this would be very useful.

I'm not quite sure how it would be for larger apps that use POP3
somewhere (I've not written any of those), but if it was a library I was
responsible for, I would probably at least include a factory method
based on named parameters as a convenience utility.

And just to repeat: This is just input/opinions, it is your software,
and I trust you to make the appropriate choice.

Eivind.