lucenenet icon indicating copy to clipboard operation
lucenenet copied to clipboard

SWEEP: Add string overloads to eliminate unnecessary allocations of FileInfo and DirectoryInfo objects

Open NightOwl888 opened this issue 1 year ago • 2 comments

In Java it is common to use the File or Path object as a parameter to methods, as they are accepted as parameters of stream constructors and many other places.

However, in .NET, the corresponding objects (TextWriter, Stream, StreamWriter, etc) don't accept a FileInfo as a parameter, they accept strings. As such, we are commonly newing up these FileInfo objects, passing them through our API and reading the original string back as FileInfo.FullName. The FileInfo object is not even used in many cases.

To keep the API the same as Lucene, we should keep the overloads that accept FileInfo, but add overloads that are identical in every way except that they accept a string for a file name instead of a FileInfo. The FileInfo overloads can then just pass FileInfo.FullName to the string overloads.

For example, we currently have

        /// <summary>
        /// Returns an <see cref="Stream"/> over the requested file, identifying
        /// the appropriate <see cref="Stream"/> instance similar to <see cref="GetInputStream(FileInfo)"/>.
        /// </summary>
        public static Stream GetOutputStream(FileInfo file)
        {
            // First, create a FileInputStream, as this will be required by all types.
            // Wrap with BufferedInputStream for better performance
            Stream os = new FileStream(file.FullName, FileMode.Create, FileAccess.ReadWrite, FileShare.ReadWrite);
            return GetFileType(file).GetOutputStream(os);
        }

And we should change it to

        /// <summary>
        /// Returns an <see cref="Stream"/> over the requested <paramref name="file"/>, identifying
        /// the appropriate <see cref="Stream"/> instance similar to <see cref="GetInputStream(FileInfo)"/>.
        /// </summary>
        public static Stream GetOutputStream(FileInfo file)
            => GetOutputStream(file?.FullName);

        /// <summary>
        /// Returns an <see cref="Stream"/> over the requested <paramref name="filePath"/>, identifying
        /// the appropriate <see cref="Stream"/> instance similar to <see cref="GetInputStream(string)"/>.
        /// </summary>
        public static Stream GetOutputStream(string filePath)
        {
            // First, create a FileInputStream, as this will be required by all types.
            // Wrap with BufferedInputStream for better performance
            Stream os = new FileStream(filePath, FileMode.Create, FileAccess.ReadWrite, FileShare.ReadWrite);
            return GetFileType(filePath).GetOutputStream(os);
        }

There are several places throughout the project where we are instantiating and then discarding FileInfo objects without even making use of their functionality (example). Some could instead use static methods (i.e. File.Exists). In some other cases, these FileInfo and DirectoryInfo instances are appropriate.

NightOwl888 avatar Apr 13 '23 18:04 NightOwl888