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:
> 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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131085/new/
https://reviews.llvm.org/D131085
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
