optparse alternative

Henry Ludemann lists at hl.id.au
Mon Mar 14 14:13:18 EST 2005


>
> * The more I think about it, the more I like that you're basically 
> constructing options to match a function signature.  But I can imagine 
> that this might be too restrictive for some uses.  It might be worth 
> having an alternate interface that provides an options object like 
> optparse does.


It is matching options to a function signiture, and for complex methods, 
this might be too restrictive (for example, you'd get methods with 10 - 
20 params). That said, many cases don't have this many things that can 
be set.

Although your point about providing an alternate interface is a good one...

> * Any reason why you aren't using new-style classes?


Ignorance. I'll look into it...

> * I think the code would be easier to navigate in a single module.


It might be easier to navigate (eg: when you want to go to a method 
implmentation), but from a maintenance point of view I feel modular code 
is easier... Mostly this is instint from other languages (mostly c++).

> * Your code alternates between underscore_names and camelCaseNames. 
> Might be better to stick with one or the other.  (PEP 8 suggests the 
> former.)


Yeah, I coded it before I looked at PEP8. Initially it was all camel 
case, and I attempted to retrofit it. It seems like classes should still 
be CamelCase, yes?

> * File specific comments:
>
> Argument.py:
> Drop the get_name method; name attribute is already accessible.  (The 
> property builtin makes getters and setters generally unnecessary.)
>
> CommandArgument.py:
> Drop the get_param_name method; param_name attribute is already 
> accessible
>
> Flag.py:
> __init__ should have defaults where applicable (e.g. the comments say 
> you can provide None for short_flag, but it doesn't default to this). 
> Should also probably produce an error if neither short_flag nor 
> long_flag is set.  (Or do this in Command._add_options)
>
> In get_value, use "self.argument is None" not "self.argument == None".
>
> get_flag_name should default to long_flag, not short_flag
>
> Command.py:
> add_flag should call _validate_param_name *before* _add_options (in 
> case an exception is raised and caught).
>
> In _get_params,
>     for arg_index in range(len(method_args)):
>         arg = method_args[arg_index]
> could be replaced with
>     for arg_index, arg in enumerate(method_args):


Will do...

>
> MulipleLines.py:
> Can you use the (standard library) textwrap module instead?  Seems 
> like they do about the same thing, but I haven't looked in too much 
> detail.


Doh! Yep, they do the same thing. This is another case of my not 
checking the standard library well enough.

> Hope these comments are at least marginally useful. ;)


Yes, very useful. Thanks very much for spending the time to go over it...




More information about the Python-list mailing list