Refactor temporary file handling for efficiency #1865
Conversation
| } | ||
| _instance = null; | ||
| return null; |
There was a problem hiding this comment.
Can't you just delete this line now?
There was a problem hiding this comment.
The linter complains if I do that; I can leave the return in, or add a lint exclusion.
|
|
||
| Future<File> _createTemporaryFile() async { | ||
| int _temporaryFileCount = 0; | ||
| Future<File> createTemporaryFile() async { | ||
| _temporaryFileCount++; | ||
| File tempFile = new File(pathLib.join( |
There was a problem hiding this comment.
Couldn't this just use createTemp on a Directory to avoid needing to keep track of the _temporaryFileCount? I suppose it doesn't matter that much, but it seemed like what createTemp exists to do.
There was a problem hiding this comment.
I suppose it could. createTemp however is a bit overkill as it is intended to prevent collisions between applications, not just within the same application. Having the integer numbers in the name I actually like, since it makes it a little easier to see what's going on under the hood.
Fixes a rare problem where incrementing the file counter asynchronously with determining the filename could result in collisions. Also combines all tool temporary files into a single manager, and reduces the number of ToolRunner objects by pairing them directly with ToolConfiguration objects. This means no longer creating and then destroying a directory for each tool-using ModelElement. Reduces runtime another 10 percent or so for Flutter.