On 2021-09-30 11:23 a.m., Chris Angelico wrote: > > For example this: (real code) > > > > def get_property_values(self, prop): > > try: > > factory = self.get_supported_properties()[prop] > > except KeyError as exc: raise PropertyError from exc > > iterator = factory(self._obj) > > try: > > first = next(iterator) > > except StopIteration: return (x for x in ()) > > except abdl.exceptions.ValidationError as exc: raise LookupError > > from exc > > except LookupError as exc: raise RuntimeError from exc # don't > > accidentally swallow bugs in the iterator > > return itertools.chain([first], iterator) > > There's a lot of exception transformation happening here, and I don't > understand, without context, the full purpose of it all. But the fact > that you're raising LookupError directly seems a tad odd (it's a > parent class for IndexError and KeyError), and it seems like a custom > exception type would solve all of this. I *think* what's happening > here is that you transform ValidationError into LookupError, but only > if it happens on the very first yielded result?? Then how about: > > class PropertyNotFoundError(LookupError): pass > > def get_property_values(self, prop): > try: > factory = self.get_supported_properties()[prop] > except KeyError as exc: raise PropertyError from exc > iterator = factory(self._obj) > try: > yield next(iterator) > except StopIteration: pass > except abdl.exceptions.ValidationError as exc: > raise PropertyNotFoundError from exc > yield from iterator > > If the factory raises some other LookupError, then that's not a > PropertyNotFoundError. And if it *does* raise PropertyNotFoundError, > then that is clearly deliberate, and it's a signal that the property > is, in fact, not found. > > Your code, as currently written (and including the rewrite), is > *still* capable of leaking a faulty exception - just as long as the > factory returns one good value first. So I'm not sure what you > actually gain by this transformation.
Okay let us stop you right there. This generator is not a function. Generators don't do anything until iterated. There's a HUGE semantic difference there. You're making assumptions that you could easily invalidate yourself if you paid attention to the original code. Don't do that. try: iterator = foo.get_property_values(prop) except PropertyError, etc: pass # handle it for thing in iterator: pass # exceptions in the iteration actually get propagated because they mean something went wrong and the program should terminate. in the real code, this includes spurious ValidationError etc, because those are *never* supposed to happen. _______________________________________________ Python-ideas mailing list -- python-ideas@python.org To unsubscribe send an email to python-ideas-le...@python.org https://mail.python.org/mailman3/lists/python-ideas.python.org/ Message archived at https://mail.python.org/archives/list/python-ideas@python.org/message/CPXRLKFULB44IK57SKCYV74AMY3GXDYE/ Code of Conduct: http://python.org/psf/codeofconduct/
