Excerpts from Yuya Nishihara's message of 2017-02-05 13:59:32 +0900:
> Perhaps that is a matter of taste, I like the explicit one. But I heard
> Python 3 has a generator-based coroutine, so using "yield" for that purpose
> is more common in Python 3?
>
> @preload('index')
> def preloadindex(repo):
> f = repo.svfs('changelog.i') # "f" can be shared naturally
> hash = fstat(f).st_size
> def loadindex():
> indexdata = f.read()
> hash = len(indexdata)
> return parseindex(indexdata), hash
The inner "loadindex" function makes things unnecessarily more complex in my
opinion. It makes the final return type harder to read, introduces extra
indentation and a function definition.
If the goal is to get rid of "yield", it could be done in different ways.
For example,
Choice A: explicitly pass oldhash and the oldobject. We probably need
the old object anyway to allow incremental updates:
@preload('obsstore', depend=['changelog'])
def preloadobsstore(repo, oldhash, oldobsstore):
obsstore = oldobsstore
newhash = ...
if newhash != oldhash:
# note: this is an indentation not existed using yield
obsstore = ...
return newhash, obsstore
Choice B: just give the preload function access to the cache object:
@preload(['obsstore', 'changelog'])
def preloadmultiple(repo, cache):
oldhash, oldobsstore = cache['obsstore']
....
Choice A looks simple. Choice B is more expressive while the function body
could be out of control - ex. it could skip setting cache['obsstore'] even
if it says so in its decorator.
While Choice A may serve most requirements just well, I think "yield" is
more expressive, and avoid unnecessary indentation. For example, the
following could not be expressed using "return" cleanly and easily:
- Optional dependency:
if fastpath_cannot_work:
yield preload.require('changelog')
Maybe "raise MissingRequirement('changelog')", but that makes
the function "restarted", while the generator could continue
executing without losing context.
- Optionally change the cache key:
As mentioned in "4)", [1]
Therefore I think the generator way avoids all kinds of indentations
(reminds me of nodejs callback hell), and has better extensibility. So I
currently prefer it. Otherwise the "Choice A" may be good enough for now.
The code base has already used "yield" for its future implementation used
for batched remote peer calls.
[1]
https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-February/092584.html
> return hash, loadindex
>
> BTW, when will 'f' be closed if we take the generator-based abstraction?
>
> def preloadindex(repo):
> f = repo.svfs('changelog.i')
> try:
> yield ...
> yield ...
> finally:
> f.close()
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
