Skip to content

Commit 58a50a2

Browse files
droyaddroyad
committed
Change the method to return null instead of Try/Out pattern
1 parent d4edbd2 commit 58a50a2

File tree

6 files changed

+34
-62
lines changed

6 files changed

+34
-62
lines changed

‎src/Assent.Tests/DirPathTests.cs‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,44 +12,40 @@ public class NoSubDirs
1212
[Test]
1313
public void SuccessWithValueIfEnvVarSetAndDirExists()
1414
{
15-
var result = DirPath.TryGetFromEnvironment("LOCALAPPDATA", [], out var value,
15+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", [],
1616
getEnvironmentVariable: _ => "localAppDataFolder",
1717
directoryExists: _ => true);
1818

19-
result.Should().BeTrue();
2019
value.Should().Be("localAppDataFolder");
2120
}
2221

2322
[Test]
2423
public void FailIfEnvVarNotSet()
2524
{
26-
var result = DirPath.TryGetFromEnvironment("LOCALAPPDATA", [], out var value,
25+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", [],
2726
getEnvironmentVariable: _ => null,
2827
directoryExists: _ => throw new InvalidOperationException("should not get here"));
2928

30-
result.Should().BeFalse();
3129
value.Should().BeNull();
3230
}
3331

3432
[Test]
3533
public void FailIfEnvVarIsEmpty()
3634
{
37-
var result = DirPath.TryGetFromEnvironment("LOCALAPPDATA", [], out var value,
35+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", [],
3836
getEnvironmentVariable: _ => "",
3937
directoryExists: _ => throw new InvalidOperationException("should not get here"));
4038

41-
result.Should().BeFalse();
4239
value.Should().BeNull();
4340
}
4441

4542
[Test]
4643
public void FailIfEnvVarSetAndDirDoesNotExist()
4744
{
48-
var result = DirPath.TryGetFromEnvironment("LOCALAPPDATA", [], out var value,
45+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", [],
4946
getEnvironmentVariable: _ => "localAppDataFolder",
5047
directoryExists: _ => false);
5148

52-
result.Should().BeFalse();
5349
value.Should().BeNull();
5450
}
5551
}
@@ -59,40 +55,37 @@ public class CombiningSubDirs
5955
[Test]
6056
public void SuccessWithValueIfEnvVarSetAndDirExists()
6157
{
62-
var result = DirPath.TryGetFromEnvironment("LOCALAPPDATA", ["foo", "bar"], out var value,
58+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", ["foo", "bar"],
6359
getEnvironmentVariable: _ => "localAppDataFolder",
6460
directoryExists: _ => true);
6561

66-
result.Should().BeTrue();
6762
value.Should().Be(Path.Combine("localAppDataFolder", "foo", "bar"));
6863
}
6964

7065
[Test]
7166
public void FailIfEnvVarNotSet()
7267
{
73-
var result = DirPath.TryGetFromEnvironment("LOCALAPPDATA", ["foo", "bar"], out var value,
68+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", ["foo", "bar"],
7469
getEnvironmentVariable: _ => null,
7570
directoryExists: _ => throw new InvalidOperationException("should not get here"));
7671

77-
result.Should().BeFalse();
7872
value.Should().BeNull();
7973
}
8074

8175
[Test]
8276
public void FailIfEnvVarIsEmpty()
8377
{
84-
var result = DirPath.TryGetFromEnvironment("LOCALAPPDATA", ["foo", "bar"], out var value,
78+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", ["foo", "bar"],
8579
getEnvironmentVariable: _ => "",
8680
directoryExists: _ => throw new InvalidOperationException("should not get here"));
8781

88-
result.Should().BeFalse();
8982
value.Should().BeNull();
9083
}
9184

9285
[Test]
9386
public void FailIfEnvVarSetAndDirDoesNotExist()
9487
{
95-
var result = DirPath.TryGetFromEnvironment("LOCALAPPDATA", ["foo", "bar"], out var value,
88+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", ["foo", "bar"],
9689
getEnvironmentVariable: _ => "localAppDataFolder",
9790
directoryExists: d =>
9891
{
@@ -101,7 +94,6 @@ public void FailIfEnvVarSetAndDirDoesNotExist()
10194
return false;
10295
});
10396

104-
result.Should().BeFalse();
10597
value.Should().BeNull();
10698
}
10799
}

‎src/Assent/Assent.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,6 @@
1111
<PackageProjectUrl>https://github.com/droyad/Assent</PackageProjectUrl>
1212
<RepositoryUrl>https://github.com/droyad/Assent</RepositoryUrl>
1313
<PackageLicenseUrl>https://github.com/droyad/Assent/blob/master/LICENCE.md</PackageLicenseUrl>
14+
<LangVersion>latest</LangVersion>
1415
</PropertyGroup>
1516
</Project>

‎src/Assent/DirPath.cs‎

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,34 @@
11
using System;
2-
using System.Diagnostics.CodeAnalysis;
32
using System.IO;
3+
using System.Linq;
44

55
namespace Assent
66
{
77
internal static class DirPath
88
{
9-
/// <summary>
10-
/// Reads the environment variable defined by `name`, and checks that it exists using Directory.Exists.
11-
/// If no such directory exists, will return false and `value` will be null. If it does exist, will return true and `value` will be the environment variable value.
12-
/// </summary>
13-
internal static bool TryGetFromEnvironment(string name, [MaybeNullWhen(returnValue: false)] out string value)
14-
=> TryGetFromEnvironment(name, Array.Empty<string>(), out value);
15-
169
/// <summary>
1710
/// Reads the environment variable defined by `name`, appends the given subdirectories from `combineSubDirs` and checks that it exists using Directory.Exists.
18-
/// If no such directory exists, will return false and `value` will be null. If it does exist, will return true and `value` will be the environment variable value.
11+
/// If no such directory exists, will return null. If it does exist, will return the environment variable value.
1912
/// </summary>
20-
internal static bool TryGetFromEnvironment(string name, string[] combineSubDirs, [MaybeNullWhen(returnValue: false)] out string value)
21-
=> TryGetFromEnvironment(name, combineSubDirs, out value, Environment.GetEnvironmentVariable, Directory.Exists);
13+
internal static string GetFromEnvironmentOrNull(string name, params string[] combineSubDirs)
14+
=> GetFromEnvironmentOrNull(name, combineSubDirs, Environment.GetEnvironmentVariable, Directory.Exists);
2215

2316
// This method allows injected I/O functions for unit testing. Don't call it outside of tests
24-
internal static bool TryGetFromEnvironment(
17+
internal static string GetFromEnvironmentOrNull(
2518
string name,
2619
string[] combineSubDirs,
27-
[MaybeNullWhen(returnValue: false)] out string value,
2820
Func<string, string> getEnvironmentVariable,
2921
Func<string, bool> directoryExists)
3022
{
31-
var valueFromEnv = getEnvironmentVariable(name);
32-
if (string.IsNullOrWhiteSpace(valueFromEnv))
33-
{
34-
value = null;
35-
return false;
36-
}
37-
38-
if (combineSubDirs is { Length: > 0 })
39-
{
40-
var components = new string[combineSubDirs.Length + 1];
41-
components[0] = valueFromEnv;
42-
Array.Copy(combineSubDirs, 0, components, 1, combineSubDirs.Length);
43-
value = Path.Combine(components);
44-
}
45-
else
46-
{
47-
value = valueFromEnv;
48-
}
23+
var pathFromEnv = getEnvironmentVariable(name);
24+
if (string.IsNullOrWhiteSpace(pathFromEnv))
25+
return null;
4926

50-
if (directoryExists(value)) return true;
27+
var pathWithSubDirs = Path.Combine(new[] {pathFromEnv}.Concat(combineSubDirs).ToArray());
5128

52-
value = null;
53-
return false;
29+
return directoryExists(pathWithSubDirs)
30+
? pathWithSubDirs
31+
: null;
5432
}
5533
}
5634
}

‎src/Assent/Reporters/DiffPrograms/DiffProgramBase.cs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,15 @@ public abstract class DiffProgramBase : IDiffProgram
1010
{
1111
protected static IReadOnlyList<string> WindowsProgramFilePaths()
1212
{
13-
var result = new List<string>();
14-
15-
if (DirPath.TryGetFromEnvironment("ProgramFiles", out var pf)) result.Add(pf);
16-
17-
if (DirPath.TryGetFromEnvironment("ProgramFiles(x86)", out var pf86) && !result.Contains(pf86)) result.Add(pf86);
18-
if (DirPath.TryGetFromEnvironment("ProgramW6432", out var pfw64) && !result.Contains(pfw64)) result.Add(pfw64);
19-
20-
if (DirPath.TryGetFromEnvironment("LocalAppData", new[] { "Programs" }, out var appDataPrograms)) result.Add(appDataPrograms);
13+
var result = new[]
14+
{
15+
DirPath.GetFromEnvironmentOrNull("ProgramFiles"),
16+
DirPath.GetFromEnvironmentOrNull("ProgramFiles(x86)"),
17+
DirPath.GetFromEnvironmentOrNull("ProgramW6432"),
18+
DirPath.GetFromEnvironmentOrNull("LocalAppData", "Programs")
19+
};
2120

22-
return result;
21+
return result.Where(r => r != null).ToArray();
2322
}
2423

2524
public IReadOnlyList<string> SearchPaths { get; }

‎src/Assent/Reporters/DiffPrograms/RiderDiffProgram.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ public class RiderDiffProgram : DiffProgramBase
1212

1313
static RiderDiffProgram()
1414
{
15-
if (!DirPath.TryGetFromEnvironment("LocalAppData", new[] { "JetBrains", "Toolbox", "apps", "Rider" }, out var riderChannelsDirectory))
15+
var riderChannelsDirectory = DirPath.GetFromEnvironmentOrNull("LocalAppData", "JetBrains", "Toolbox", "apps", "Rider");
16+
if (riderChannelsDirectory == null)
1617
{
17-
DefaultSearchPaths = Array.Empty<string>();
18+
DefaultSearchPaths = [];
1819
return;
1920
}
2021

‎src/Assent/Reporters/DiffPrograms/XdiffDiffProgram.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ public class XdiffDiffProgram : IDiffProgram
99
{
1010
static XdiffDiffProgram()
1111
{
12-
if (DirPath.TryGetFromEnvironment("LocalAppData", new[] { "semanticmerge " }, out var semanticMergeDir))
12+
var semanticMergeDir = DirPath.GetFromEnvironmentOrNull("LocalAppData", "semanticmerge");
13+
if (semanticMergeDir != null)
1314
{
1415
InstallPath = Path.Combine(semanticMergeDir, "mergetool.exe");
1516
}

0 commit comments

Comments
 (0)