Greg Ewing wrote:
Jacob Holm wrote:
I am saying that there are examples where it is desirable to move one of the arguments that this form of refactoring forces you to put in the constructor so it instead becomes the argument of the first send.
I'm having trouble seeing circumstances in which you would need to do that. Can you provide an example in the form of
(a) a piece of unfactored code
Ok, once again based on your own parser example. The parse_items generator could have been written as: def parse_items(closing_tag = None): elems = [] token = yield while token != closing_tag: if is_opening_tag(token): name = token[1:-1] items = yield from parse_items("%s>" % name) elems.append((name, items)) else: elems.append(token) token = yield return elems
(b) a desired refactoring
I would like to split off a function for parsing a single element. And I would like it to look like this: def parse_elem(): opening_tag = yield name = opening_tag[1:-1] items = yield from parse_items("%s>" % name) return (name, items) This differs from the version in your example by taking all the tags as arguments to send() instead of having the opening tag as an argument to the constructor. Unfortunately, there is no way to actually use this version in the implementation of parse_items.
(c) an explanation of why the desired refactoring can't conveniently be done using an unprimed generator and plain yield-from.
The suggested subroutine cannot be used, because parse_items already has the value that should go as the argument to the first send(). It is easy to rewrite it to the version you used in the example, but that requires you to make the opening_tag an argument to the constructor, whereas I want it as an argument to the first send. You can of course make that argument optional and adjust the function to only do the first yield if the argument is not given. That is essentially what my "cr_init()" pattern does. Using that pattern, the refactoring looks like this: def cr_init(start): if start is None: return yield if 'send' in start: return start['send'] if 'throw' in start: raise start['throw'] return yield start.get('yield') def parse_elem(start=None): opening_tag = yield from cr_init(start) name = opening_tag[1:-1] items = yield from parse_items("%s>" % name) return (name, items) def parse_items(closing_tag=None, start=None): elems = [] token = yield from cr_init(start) while token != closing_tag: if is_opening_tag(token): elems.append(yield from parse_elem(start={'send':token})) else: elems.append(token) token = yield return elems As you see it *can* be done, but I would hardly call it convenient. The main problem is that the coroutine you want to call must be written with this in mind or you are out of luck. While it *is* possible to write a wrapper that lets you call the unmodified parse_elem, that wrapper cannot use yield_from to call it so you get a rather large overhead that way. A convention like Nick suggested where all coroutines take an optional "start" argument with the first value to yield doesn't help, because it is not the value to yield that is the problem. I hope this helps to explain why the cr_init pattern is needed even for relatively simple refactoring now that it seems we are not fixing the "initial next()" issue. - Jacob