[Lldb-commits] [PATCH] D68188: allow arbitrary python streams to be converted to SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D68188#1687532 <https://reviews.llvm.org/D68188#1687532>, @labath wrote:

> It seems to be that instead of subclassing a fully-functional File class, it would be better to create an abstract class, that both `File` and the new python thingy could implement. That would also create a natural place to write down the FILE* conversion semantics and other stuff we talked about at lldb-dev. I don't have any opinion as to whether the name `File` should denote the new abstract class, or the real thing, but it should most likely be done as a separate patch.
>
> What do you think about that?


I could do it that way, but I have some reservations about it.    In this version the class hierarchy looks like this:

            File
             |
   SimplePythonFile --------+
    |                       |
  TextPythonFile     BinaryPythonFile

If we added an abstract base class, I guess it would look like this:

            AbstractFile
             |        |                
            File      PythonFile
             |         |     | |
           SimplePythonFile  | |
                             | |
    + -----------------------+ |  
    |                          |
  TextPythonFile     BinaryPythonFile

Does a more complicated class hierarchy like that really solve a problem or make the code easier to understand?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68188/new/

https://reviews.llvm.org/D68188