JDevlieghere added inline comments.
================ Comment at: lldb/examples/python/crashlog.py:434 except CrashLogFormatException: - return TextCrashLogParser(debugger, path, verbose).parse() + return object().__new__(TextCrashLogParser) ---------------- 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? 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