Skip to content

Commit

Permalink
Improve Code Quality (#80)
Browse files Browse the repository at this point in the history 
* Make TextSlice readonly

* Update IFileSystem to use file-scoped namespaces

* Make NodeResult readonly and pass by reference

* Replace NET45 symbol with NETFRAMEWORK

* Remove redundant scope

* Eliminate a few allocations in ZipArchiveFileSystem and seal private classes

* Improve nullability annotations
  • Loading branch information
@iamcarbon
iamcarbon authored Dec 9, 2023
1 parent 468c5e8 commit fde2ba3
Show file tree
Hide file tree
Showing 8 changed files with 270 additions and 280 deletions.
2 changes: 1 addition & 1 deletion src/Zio/FileSystemEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public override string ToString()
}

/// <inheritdoc />
public bool Equals(FileSystemEntry other)
public bool Equals(FileSystemEntry? other)
{
if (other is null) return false;
if (ReferenceEquals(this, other)) return true;
Expand Down
16 changes: 7 additions & 9 deletions src/Zio/FileSystemExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,11 @@ public static byte[] ReadAllBytes(this IFileSystem fs, UPath path)
public static string ReadAllText(this IFileSystem fs, UPath path)
{
var stream = fs.OpenFile(path, FileMode.Open, FileAccess.Read, FileShare.Read);

using (var reader = new StreamReader(stream))
{
using (var reader = new StreamReader(stream))
{
return reader.ReadToEnd();
}
}
return reader.ReadToEnd();
}
}

/// <summary>
Expand All @@ -359,11 +358,10 @@ public static string ReadAllText(this IFileSystem fs, UPath path, Encoding encod
{
if (encoding is null) throw new ArgumentNullException(nameof(encoding));
var stream = fs.OpenFile(path, FileMode.Open, FileAccess.Read, FileShare.Read);

using (var reader = new StreamReader(stream, encoding))
{
using (var reader = new StreamReader(stream, encoding))
{
return reader.ReadToEnd();
}
return reader.ReadToEnd();
}
}

Expand Down
13 changes: 6 additions & 7 deletions src/Zio/FileSystems/MemoryFileSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ protected override IEnumerable<UPath> EnumeratePathsImpl(UPath path, string sear
// and the time we are going to actually visit it, it might have been
// removed in the meantime, so we make sure here that we have a folder
// and we don't throw an error if it is not
if (!(result.Node is DirectoryNode))
if (result.Node is not DirectoryNode)
{
continue;
}
Expand Down Expand Up @@ -885,7 +885,7 @@ protected override IEnumerable<FileSystemItem> EnumerateItemsImpl(UPath path, Se
// and the time we are going to actually visit it, it might have been
// removed in the meantime, so we make sure here that we have a folder
// and we don't throw an error if it is not
if (!(result.Node is DirectoryNode))
if (result.Node is not DirectoryNode)
{
continue;
}
Expand Down Expand Up @@ -1106,8 +1106,7 @@ void AssertNoDestination(FileSystemNode node)
}
}


private void ValidateDirectory([NotNull] FileSystemNode? node, UPath srcPath)
private static void ValidateDirectory([NotNull] FileSystemNode? node, UPath srcPath)
{
if (node is FileNode)
{
Expand All @@ -1120,7 +1119,7 @@ private void ValidateDirectory([NotNull] FileSystemNode? node, UPath srcPath)
}
}

private void ValidateFile([NotNull] FileSystemNode? node, UPath srcPath)
private static void ValidateFile([NotNull] FileSystemNode? node, UPath srcPath)
{
if (node is null)
{
Expand Down Expand Up @@ -1179,7 +1178,7 @@ private void CreateDirectoryNode(UPath path)
ExitFindNode(EnterFindNode(path, FindNodeFlags.CreatePathIfNotExist | FindNodeFlags.NodeShared));
}

private struct NodeResult
private readonly struct NodeResult
{
public NodeResult(DirectoryNode? directory, FileSystemNode node, string? name, FindNodeFlags flags)
{
Expand Down Expand Up @@ -1214,7 +1213,7 @@ private enum FindNodeFlags
KeepParentNodeShared = 1 << 6,
}

private void ExitFindNode(NodeResult nodeResult)
private void ExitFindNode(in NodeResult nodeResult)
{
var flags = nodeResult.Flags;

Expand Down
6 changes: 3 additions & 3 deletions src/Zio/FileSystems/MountFileSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public MountFileSystem(bool owned = true) : this(null, owned)
/// Initializes a new instance of the <see cref="MountFileSystem"/> class with a default backup filesystem.
/// </summary>
/// <param name="defaultBackupFileSystem">The default backup file system.</param>
/// <param name="owned">True if <paramref name="defaultBackupFileSystem"/> and mounted filesytems should be disposed when this instance is disposed.</param>
/// <param name="owned">True if <paramref name="defaultBackupFileSystem"/> and mounted filesystems should be disposed when this instance is disposed.</param>
public MountFileSystem(IFileSystem? defaultBackupFileSystem, bool owned = true) : base(defaultBackupFileSystem, owned)
{
_mounts = new SortedList<UPath, IFileSystem>(new UPathLengthComparer());
Expand Down Expand Up @@ -144,7 +144,7 @@ public IFileSystem Unmount(UPath name)
{
ValidateMountName(name);

IFileSystem mountFileSystem;
IFileSystem? mountFileSystem;

if (!_mounts.TryGetValue(name, out mountFileSystem))
{
Expand Down Expand Up @@ -1011,7 +1011,7 @@ private static UPath CombinePrefix(UPath prefix, UPath remaining)
: prefix / remaining.ToRelative();
}

private void ValidateMountName(UPath name)
private static void ValidateMountName(UPath name)
{
name.AssertAbsolute(nameof(name));
if (name == UPath.Root)
Expand Down
52 changes: 24 additions & 28 deletions src/Zio/FileSystems/ZipArchiveFileSystem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class ZipArchiveFileSystem : FileSystem
private readonly Dictionary<ZipArchiveEntry, EntryState> _openStreams;
private readonly object _openStreamsLock = new();

#if NET45 // .Net4.5 uses a backslash as directory separator
#if NETFRAMEWORK // .Net4.5 uses a backslash as directory separator
private const char DirectorySeparator = '\\';
#else
private const char DirectorySeparator = '/';
Expand All @@ -56,7 +56,7 @@ public ZipArchiveFileSystem(ZipArchive archive, bool isCaseSensitive = false, Co
{
throw new ArgumentNullException(nameof(archive));
}
#if NET45 // .Net4.5 uses a backslash as directory separator
#if NETFRAMEWORK // .Net4.5 uses a backslash as directory separator
foreach (var entry in _archive.Entries)
{
entry.FullName.Replace('/', DirectorySeparator);
Expand Down Expand Up @@ -112,7 +112,7 @@ public ZipArchiveFileSystem(ZipArchiveMode mode = ZipArchiveMode.Update, bool le

private ZipArchiveEntry? GetEntry(string path)
{
#if NET45
#if NETFRAMEWORK
path = path.Replace('/', DirectorySeparator);
#else
path = path.Replace('\\', DirectorySeparator);
Expand Down Expand Up @@ -243,7 +243,7 @@ protected override void CreateDirectoryImpl(UPath path)
}

var entryPath = RemoveLeadingSlash(path);
#if NET45
#if NETFRAMEWORK
entryPath = entryPath.Replace('/', DirectorySeparator);
#else
entryPath = entryPath.Replace('\\', DirectorySeparator);
Expand All @@ -256,7 +256,7 @@ protected override void CreateDirectoryImpl(UPath path)
protected override void DeleteDirectoryImpl(UPath path, bool isRecursive)
{
var entryPath = RemoveLeadingSlash(ConvertPathToDirectory(path));
#if NET45
#if NETFRAMEWORK
entryPath = entryPath.Replace('/', DirectorySeparator);
#else
entryPath = entryPath.Replace('\\', DirectorySeparator);
Expand Down Expand Up @@ -427,7 +427,7 @@ private IEnumerable<string> EnumeratePathsStr(UPath path, string searchPattern,
var search = SearchPattern.Parse(ref path, ref searchPattern);
var entryPath = RemoveLeadingSlash(path);
var root = ConvertPathToDirectory(entryPath);
#if NET45
#if NETFRAMEWORK
root = root.Replace('/', '\\');
#else
root = root.Replace('\\', '/');
Expand Down Expand Up @@ -484,7 +484,7 @@ protected override bool FileExistsImpl(UPath path)
protected override FileAttributes GetAttributesImpl(UPath path)
{
var entry = GetEntry(path.FullName) ?? GetEntry(ConvertPathToDirectory(path));
if (entry == null)
if (entry is null)
{
throw FileSystemExceptionHelper.NewFileNotFoundException(path);
}
Expand Down Expand Up @@ -562,15 +562,15 @@ protected override void MoveDirectoryImpl(UPath srcPath, UPath destPath)

var srcDir = RemoveLeadingSlash(ConvertPathToDirectory(srcPath.FullName));
var destDir = RemoveLeadingSlash(ConvertPathToDirectory(destPath.FullName));
#if NET45
#if NETFRAMEWORK
srcDir = srcDir.Replace('/', DirectorySeparator);
destDir = destDir.Replace('/', DirectorySeparator);
#else
srcDir = srcDir.Replace('\\', DirectorySeparator);
destDir = destDir.Replace('\\', DirectorySeparator);
#endif
_entriesLock.EnterReadLock();
var entries = new ZipArchiveEntry[0];
var entries = Array.Empty<ZipArchiveEntry>();
try
{
entries = _archive.Entries.Where(e => e.FullName.StartsWith(srcDir, _isCaseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase)).ToArray();
Expand Down Expand Up @@ -618,12 +618,8 @@ protected override void MoveDirectoryImpl(UPath srcPath, UPath destPath)
/// <inheritdoc />
protected override void MoveFileImpl(UPath srcPath, UPath destPath)
{
var srcEntry = GetEntry(srcPath.FullName);
if (srcEntry == null)
{
throw FileSystemExceptionHelper.NewFileNotFoundException(srcPath);
}

var srcEntry = GetEntry(srcPath.FullName) ?? throw FileSystemExceptionHelper.NewFileNotFoundException(srcPath);

if (!DirectoryExistsImpl(destPath.GetDirectory()))
{
throw FileSystemExceptionHelper.NewDirectoryNotFoundException(destPath.GetDirectory());
Expand All @@ -633,8 +629,7 @@ protected override void MoveFileImpl(UPath srcPath, UPath destPath)
if (destEntry != null)
{
throw new IOException("Cannot overwrite existing file.");
}

}

destEntry = CreateEntry(destPath.FullName);
TryGetDispatcher()?.RaiseCreated(destPath);
Expand Down Expand Up @@ -730,7 +725,7 @@ protected override Stream OpenFileImpl(UPath path, FileMode mode, FileAccess acc
protected override void ReplaceFileImpl(UPath srcPath, UPath destPath, UPath destBackupPath, bool ignoreMetadataErrors)
{
var sourceEntry = GetEntry(srcPath.FullName);
if (sourceEntry == null)
if (sourceEntry is null)
{
throw FileSystemExceptionHelper.NewFileNotFoundException(srcPath);
}
Expand Down Expand Up @@ -813,7 +808,7 @@ protected override void SetLastAccessTimeImpl(UPath path, DateTime time)
protected override void SetLastWriteTimeImpl(UPath path, DateTime time)
{
var entry = GetEntry(path.FullName) ?? GetEntry(ConvertPathToDirectory(path.FullName));
if (entry == null)
if (entry is null)
{
throw FileSystemExceptionHelper.NewFileNotFoundException(path);
}
Expand Down Expand Up @@ -856,7 +851,7 @@ private void RemoveEntry(ZipArchiveEntry entry)
private ZipArchiveEntry CreateEntry(string path)
{
path = RemoveLeadingSlash(path);
#if NET45
#if NETFRAMEWORK
path = path.Replace('/', DirectorySeparator);
#else
path = path.Replace('\\', DirectorySeparator);
Expand All @@ -879,17 +874,19 @@ private ZipArchiveEntry CreateEntry(string path)
}
}

private string GetName(ZipArchiveEntry entry)
private static readonly char[] s_slashChars = { '/', '\\' };

private static string GetName(ZipArchiveEntry entry)
{
var name = entry.FullName.TrimEnd('/', '\\');
var index = name.LastIndexOfAny(new[] { '/', '\\' });
var name = entry.FullName.TrimEnd(s_slashChars);
var index = name.LastIndexOfAny(s_slashChars);
return name.Substring(index + 1);
}

private static string GetParent(string path)
{
path = path.TrimEnd('/', '\\');
var lastIndex = path.LastIndexOfAny(new[] { '/', '\\' });
path = path.TrimEnd(s_slashChars);
var lastIndex = path.LastIndexOfAny(s_slashChars);
return lastIndex == -1 ? "" : path.Substring(0, lastIndex);
}

Expand Down Expand Up @@ -926,8 +923,7 @@ private static string RemoveLeadingSlash(string path)
}
}


private class ZipEntryStream : Stream
private sealed class ZipEntryStream : Stream
{
private readonly ZipArchiveEntry _entry;
private readonly ZipArchiveFileSystem _fileSystem;
Expand Down Expand Up @@ -1029,7 +1025,7 @@ public override void Close()
}
}

private class EntryState
private sealed class EntryState
{
public EntryState(FileShare share)
{
Expand Down
Loading

0 comments on commit fde2ba3

Please sign in to comment.