Issue 2318041: Python Issue 9873 Code Review (original) (raw)

Overall the patch looks ok, although I can't say the implementation strategy makes for very readable code.

(I would suggest to temporarily decode bytes using latin1, but the properties on ResultMixin wouldn't make that very pretty either)

http://codereview.appspot.com/2318041/diff/1/Lib/urllib/parse.py File Lib/urllib/parse.py (right):

http://codereview.appspot.com/2318041/diff/1/Lib/urllib/parse.py#newcode48 Lib/urllib/parse.py:48: uses_params = ['ftp', 'hdl', 'prospero', 'http', 'imap', Not directly related to the patch, but shoudn't all these lists be sets? It would speed up containment tests quite a bit.

http://codereview.appspot.com/2318041/diff/1/Lib/urllib/parse.py#newcode139 Lib/urllib/parse.py:139: slots = () Again, not related to the patch, but ResultMixin lacks a slots, so SplitResult will have a dict anyway.