[Webkit-unassigned] [Bug 60869] Convert json_results_generator.py to output version 4 JSON.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 24 11:22:17 PDT 2011


https://bugs.webkit.org/show_bug.cgi?id=60869





--- Comment #29 from Ojan Vafai <ojan at chromium.org>  2011-05-24 11:22:17 PST ---
(From update of attachment 94374)
View in context: https://bugs.webkit.org/attachment.cgi?id=94374&action=review

>>>>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator_unittest.py:177
>>>>> +        sub_trie = trie
>>>> 
>>>> Huh?  Why not just name the incoming argument as sub_trie?
>>> 
>>> FUD on my part about the way python passes arguments. I don't feel comfortable assigning to arguments (sub_trie is modified on line 181).
>> 
>> My understanding is that python is like javascript and does not deep-copy objects when passed.  Your sub_trie = tree also doesn't deep copy. :) so you're just modifying trie all the same!
> 
> Right, but the thing I can't cope with is having an argument which refers to a thing which was passed in, then changing what that argument refers to -- without changing the original object. I think it makes the code confusing to read.
> 
> In this case, I have a trie (more correctly, a reference to the top node of the trie) that I want to find something in, which I do by iteratively going down the trie, keeping a reference to the sub_trie I'm up to -- so the code reflects that. If I had code that took a sub_trie argument (which was really the top node of a trie, not a sub_trie at all), repeatedly assigned something else to that name and then returned something with the same name as the original argument, I think it would be very hard to work out what that did.

Meh. In my experience of python and javascript, it's common to assign to an argument. If I were writing this, I'd keep the variable named "trie" and just assign to it. It makes the code more terse and is just as readable IMO. I don't think it's worth blocking getting this checked in, but it would be better if you could change it.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list