On Sun, Apr 19, 2009 at 1:38 AM, Mart Sõmermaa <mrts.py...@gmail.com> wrote:
> On Sun, Apr 19, 2009 at 2:06 AM, Nick Coghlan <ncogh...@gmail.com> wrote:
>> That said, I'm starting to wonder if an even better option may be to
>> just drop the kwargs support from the function and require people to
>> always supply a parameters dictionary. That would simplify the signature
>> to the quite straightforward:
>>
>>  def add_query_params(url, params, allow_dups=True, sep='&')
>
> That's the most straightforward and I like this more than the one below.
>
>> I agree that isn't a good option, but mapping True/False/None to those
>> specific behaviours also seems rather arbitrary (specifically, it is
>> difficult to remember which of "allow_dups=False" and "allow_dups=None"
>> means to ignore any duplicate keys and which means to ignore only
>> duplicate items).
>
> I'd say it's less of a problem when using named arguments, i.e. you read it 
> as:
>
> allow_dups=True : yes
> allow_dups=False : effeminately no :),
> allow_dups=None : strictly no
>
> which more or less corresponds to the behaviour.
>
>> It also doesn't provide a clear mechanism for
>> extension (e.g. what if someone wanted duplicate params to trigger an
>> exception?)
>>
>> Perhaps the extra argument should just be a key/value pair filtering
>> function, and we provide functions for the three existing behaviours
>> (i.e. allow_duplicates(), ignore_duplicate_keys(),
>> ignore_duplicate_items()) in the urllib.parse module.
>
> This would be the most flexible and conceptually right (ye olde
> strategy pattern), but would clutter the API.
>
>> Note that your implementation and docstring currently conflict with each
>> other - the docstring says "pass them via a dictionary in second
>> argument:" but the dictionary is currently the third argument (the
>> docstring also later refers to passing OrderedDictionary as the second
>> argument).
>
> It's a mistake that exemplifies once again that positional args are awkward 
> :).
>
> ---
>
> So, gentlemen, either
>
> def add_query_params(url, params, allow_dups=True, sep='&')
>
> or
>
> def allow_duplicates(...)
>
> def remove_duplicate_values(...)
>
> ...
>
> def add_query_params(url, params, strategy=allow_duplicates, sep='&')

+1 for the strategy approach.

Steve
-- 
I'm not *in*-sane. Indeed, I am so far *out* of sane that you appear a
tiny blip on the distant coast of sanity.
        --- Bucky Katt, Get Fuzzy
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Reply via email to