Gregory (Grisha) Trubetskoy wrote:
Having looked at the FieldStorage code, I'm guessing the idea was that
you parse fields as they come in and append them to a list. This
preserves the original order of fields, in case it is needed.
I assumed that as well, but I'm not sure getting the fields in a
particular order is the most common use case - not for me anyway. Plus,
I'm not suggesting getting rid of access to FieldStorage.list. For the
dict-like such as fs.keys(), I don't think people should expect keys in
any particular order. If they really want that feature then they can
subclass write their own ordered dict class.
I'm not sure that maintaining a dictionary alongside the list is the
right thing to do. It might be, but there are some difficult questions
to answer -e.g. how costly is a sequential search, and is the code
complexity (and fieldstorage code is no picnic to read as it is) worth
the speedup?
The current code is a litte hairy - it helps to drink a cup of strong
coffee before reading it. We can always hide the complexity in a
separate add_field method. As to the performance tradeoffs, I guess we
benchmark and see? I love doing benchmarks. The 2 things that bring joy
to my heart are benchmarks and unit tests. And good documentation. The
*3* things that bring joy to my heart are... well you get the idea.
Also while it would speed up retrieval, it will slow down the "write"
operation - when a field is added to fieldstorage you now need to append
it to the list, AND check whether it exists in the dictionary, then add
it there as well.
How often do developers access form fields via __getitem__? I noticed
the publisher does not use it - it iterates the list, so nothing would
be gained there.
For myself, I use it (almost?) exclusively as a dict. As for the use in
publisher, maybe that implementation needs to be examined as well. ;)
Also, something else to consider - is there a simple programatic
solution that could be documented, e.g. something like
my_fs = util.FieldStorage(req)
dict_fs = {}
dict_fs.update(my_fs)
[have no idea whether this will work :-)]
Nope. If you have multiple fields with the same name you'll lose all but
the last field. (eg. The checkbox example example on the mod_python list
that got me started on this in the first place).
and voila - you've got a dictionary based fieldstorage?
Except that FieldStorage is already supposed to be dict-like so why
would I want to duplicate the effort in my code? For example 7 out of 10
the fs methods are there to support dict-like behaviour and the other
3 are initialization helpers which will never be called by user code
anyway. To me, that makes it a dictionary. I'm not talking about adding
new functionality to FieldStorage, just examining the current
implementation wrt to performance.
Anyway, just a few cents from me.
I don't want you to think I'm hung up on this issue. It just seems to me
that the goal of mod_python moving forward should be stability, speed,
efficiency while keeping feature creep to a minumum. I think it's
worthwhile to examine existing code as we go along to see if we are
meeting these goals. We still need to have *some* code to chew on every
now and then after all. :)
Jim
