Open matthijseikelenboom opened 7 months ago
I propose a simple idea for an extensible solution: to use a node plus attachment model, much like the node tree hierarchy of most game engines.
A primary node would represent a file or folder with basic properties (e.g., size, creation date, modification date, etc.). Each node can then be extended (or decorated) with domain-specific data like change monitoring, git information, etc.
Swift's facilities, like generics, can be used to make this extended data easily accessible.
One downside of this approach is that the code requiring specific extended data must be guarded with existence checks, which can become a nuisance. This can be mitigated by throwing errors for non-existent data and using protocols that represent a "generic" node as one known to contain the required extended data.
As a CE developer I want a reliable foundation that handles and manages files Because it is crucial to other functionality of the application
Our current solution for file managements is reaching its limits and there are some serious bugs present. There has recently been an attempt at writing tests for this part of the code, but it revealed that there is unwanted functionality and results.
Acceptance criteria
Scope
In scope
Out of scope
Requirements (Might need further refinement)
Risks
CEWorkspaceFileManager
andCEWorkspaceFile
are referenced at so many places in the codebase, truly refactoring the code might prove difficult. Especially since Xcode's code analysis is plain bad.Implementation design
Problems with current implementation
There are likely multiple ways to tackle this problem, in terms of using different Apple/Swift APIs and how we design the system ourselves. The latter is the most important and something that we can control. The current setup of
CEWorkspaceFileManager
andCEWorkspaceFile
is not working properly. There are multiple cases where an "IllegalState" is achievable.One of the reasons this occurs is because currently the CEWorkspaceFile contains all kinds of information, it contains multiple variables & functions related to Git, some that have todo with UI and some that have to do with tab related stuff (Which arguably is also UI). It's not clear what the responsibility of the class is. It doesn't look like the class isn't following the single responsibility principle anymore. Everything that is somewhat related to it, gets "dumped" in there. It also has some properties that in theory have something to do with it, but from a system design point of view, they don't. An example is
fileDocument
, from the typeCodeFileDocument
. You can argue that it should be there because it represents the contents of the file and is therefore related to it. But it actually has more to do with the editor and truly modifying the contents of file. AndCEWorkspaceFile
represents the file in things like the project navigator, where the contents of the files don't matter.Another thing that can (and should) be looked at is abstraction. The current
CEWorkspaceFileManager
andCEWorkspaceFile
is referenced everywhere, and is therefore hard coupled to almost everything is the codebase. This also makes it more difficult to make unit tests, as the FileManager can not be mocked or stubbed. It also is a disadvantage in cases like these, be it requires changing all the files/components that rely on it.Things that are interesting to look into when redesigning
Right now there is one
CEWorkspaceFile
, which can be either a Folder or a File. Perhaps this is the right way to do and it is how we want it, but another possibility is to make a distinction between the two by making separate classes.If PoCs are made, it is advised to develop them test driven. Since this is core functionality of the application, we should have tests in place from the get go. This way we know how we want it to behave, before it has been coded.
Tasks