mib marked 2 inline comments as done.
mib added inline comments.
================
Comment at: lldb/examples/python/crashlog.py:434
except CrashLogFormatException:
- return TextCrashLogParser(debugger, path, verbose).parse()
+ return object().__new__(TextCrashLogParser)
----------------
JDevlieghere wrote:
> kastiglione wrote:
> > mib wrote:
> > > JDevlieghere wrote:
> > > > mib wrote:
> > > > > kastiglione wrote:
> > > > > > I have not seen the `object().__new__(SomeClass)` syntax. Why is it
> > > > > > being used for `TextCrashLogParser` but not `JSONCrashLogParser`?
> > > > > > Also, `__new__` is a static method, could it be
> > > > > > `object.__new__(...)`? Or is there a subtly that requires an
> > > > > > `object` instance? Somewhat related, would it be better to say
> > > > > > `super().__new__(...)`?
> > > > > >
> > > > > > Also: one class construction explicitly forwards the arguments, the
> > > > > > other does not. Is there a reason both aren't implicit (or both
> > > > > > explicit)?
> > > > > As you know, python class are implicitly derived from the `object`
> > > > > type, making `object.__new__` and `super().__new__` pretty much the
> > > > > same thing.
> > > > >
> > > > > In this specific case, both the `TextCrashLogParser` and
> > > > > `JSONCrashLogParser` inherits from the `CrashLogParser` class, so
> > > > > `JSONCrashLogParser` will just inherits `CrashLogParser.__new__`
> > > > > implementation if we don't override it, which creates a recursive
> > > > > loop.
> > > > > That's why I'm calling the `__new__` method specifying the class.
> > > > What's the advantage of this over this compared to a factory method?
> > > > Seems like this could be:
> > > >
> > > > ```
> > > > def create(debugger, path, verbose)
> > > > try:
> > > > return JSONCrashLogParser(debugger, path, verbose)
> > > > except CrashLogFormatException:
> > > > return TextCrashLogParser(debugger, path, verbose)
> > > > ```
> > > If we make a factory, then users could still call `__init__` on
> > > `CrashLogParser` and create a bogus object. With this approach, they're
> > > forced to instantiate a CrashLogParser like any another object.
> > `CrashLogParser.__init__` could raise an exception. With intricacy of this
> > approach, maybe it's better to use a factor method combined with an
> > exception if the base class `__init__` is called.
> +1, or maybe `abc` provide a capability to achieve the same?
IMHO, having to call an arbitrary-called method (`create/make/...`) to
instantiate an object and having the `__init__` raise an exception introduces
more intricacies in the usage of this class, compared to what I'm doing.
I prefer to keep it this way since it's more natural / safe to use. If the
implementation exercises some python internal features, that's fine because
that shouldn't matter to the user.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131085/new/
https://reviews.llvm.org/D131085
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
