Skip to content

Commit 66ca6bb

Browse files
claudeDavidVeksler
authored andcommitted
Fix: Address code review feedback - correct function purity documentation
Addresses Copilot AI code review feedback from PR #3. Corrects mislabeled "pure" functions and fixes non-determinism issues. ISSUES FIXED: 1. Non-Deterministic Function - OutputFormatter.SerializeToJson: Now accepts DateTime as parameter - Makes function deterministic (same inputs = same outputs) - DateTime.Now moved to call site (WriteToFile) 2. Mislabeled Pure Functions (performing I/O) GitHelper.cs: - HasGitDirectory: "pure predicate" → "I/O operation" (calls Directory.Exists) FileUtilities.cs: - IsStreamBinary: "pure function" → "I/O operation" - HasUtf8Bom: "pure function" → "I/O operation" (reads from stream, modifies position) - HasBinaryContent: "pure function" → "I/O operation" ProjectScanner.cs: - GetProjectStructureLines: "pure recursive" → "recursive I/O" - GetFilteredEntries: "pure function" → "I/O operation" (includes console logging side effects) - EnumerateFilesRecursively: "pure recursive" → "recursive I/O" - GetFilteredFiles: "pure enumeration" → "I/O operation" - GetFilteredDirectories: "pure enumeration" → "I/O operation" - ShouldIncludeEntry: "pure predicate" → "predicate (may perform I/O)" - ShouldIncludeFile: "pure predicate" → "predicate (may perform I/O)" - ShouldIncludeDirectory: "pure predicate" → "predicate (may perform I/O)" ConfigLoader.cs: - ParseConfig: "pure function" → documents console logging side effect ContentBuilder.cs: - BuildContentSections: "pure function" → "performs I/O via scanner" OutputFormatter.cs: - ResolveOutputPath: "pure function" → "I/O operation" (calls Directory.Exists) PathResolver.cs: - GetFolderName: "pure function" → "may perform I/O" - TryGetDirectoryName: "pure function" → "may perform I/O" (creates DirectoryInfo) - ExtractRootFolderName: "pure function" → "may perform I/O" (calls Path.GetFullPath) PRINCIPLES CLARIFIED: A function is only "pure" if: 1. Returns same output for same input (deterministic) 2. Has no observable side effects 3. Does not perform I/O operations 4. Does not modify external state 5. Does not depend on external mutable state Functions that perform I/O, access file system, write to console, or depend on current time are now correctly labeled as I/O operations or documented with their side effects. This maintains functional programming benefits while being honest about where effects occur in the codebase.
1 parent 2f1439f commit 66ca6bb

7 files changed

Lines changed: 31 additions & 26 deletions

File tree

Services/ConfigLoader.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ private static Result<string> ReadConfigFile(string fileName)
4444
}
4545

4646
/// <summary>
47-
/// Pure function: parses JSON string into AppConfig.
47+
/// Parses JSON string into AppConfig.
48+
/// Includes console logging side effects on parse errors.
4849
/// </summary>
4950
private AppConfig ParseConfig(string json)
5051
{

Services/ContentBuilder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public string Build(string projectPath, AppConfig config) =>
2727
string.Join("\n", BuildContentSections(projectPath, config));
2828

2929
/// <summary>
30-
/// Pure function that generates content sections based on configuration.
30+
/// Generates content sections based on configuration (performs I/O via scanner).
3131
/// Uses declarative approach with LINQ and immutable collections.
3232
/// </summary>
3333
private IEnumerable<string> BuildContentSections(string projectPath, AppConfig config)

Services/OutputFormatter.cs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,16 @@ public string WriteToFile(string outputTarget, string content, string format, st
2929
_console.WriteLine("\n💾 Writing output...");
3030

3131
var resolvedPath = ResolveOutputPath(outputTarget, defaultFileName);
32-
var formattedContent = FormatContent(content, format);
32+
var formattedContent = FormatContent(content, format, DateTime.Now);
3333

3434
WriteFile(resolvedPath, formattedContent);
3535

3636
return resolvedPath;
3737
}
3838

3939
/// <summary>
40-
/// Pure function: resolves output path based on target type.
40+
/// I/O operation: resolves output path based on target type.
41+
/// Checks if directory exists before combining paths.
4142
/// </summary>
4243
private static string ResolveOutputPath(string outputTarget, string defaultFileName) =>
4344
Directory.Exists(outputTarget)
@@ -61,16 +62,17 @@ private static void WriteFile(string filePath, string content)
6162
/// <summary>
6263
/// Pure function: formats content based on output format.
6364
/// </summary>
64-
private static string FormatContent(string content, string format) =>
65+
private static string FormatContent(string content, string format, DateTime timestamp) =>
6566
format.Equals("json", StringComparison.OrdinalIgnoreCase)
66-
? SerializeToJson(content)
67+
? SerializeToJson(content, timestamp)
6768
: content;
6869

6970
/// <summary>
70-
/// Pure function: serializes content to JSON with timestamp.
71+
/// Pure function: serializes content to JSON with provided timestamp.
72+
/// Deterministic - same inputs always produce same output.
7173
/// </summary>
72-
private static string SerializeToJson(string content) =>
74+
private static string SerializeToJson(string content, DateTime timestamp) =>
7375
JsonSerializer.Serialize(
74-
new { content, timestamp = DateTime.Now },
76+
new { content, timestamp },
7577
new JsonSerializerOptions { WriteIndented = true });
7678
}

Services/PathResolver.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ private static string SelectPath(string userInput, string defaultPath) =>
5454
string.IsNullOrWhiteSpace(userInput) ? defaultPath : userInput;
5555

5656
/// <summary>
57-
/// Pure function: extracts a clean folder name from the input path for output file naming.
58-
/// Uses functional composition to handle edge cases.
57+
/// Extracts a clean folder name from the input path for output file naming.
58+
/// Uses functional composition to handle edge cases. May perform I/O.
5959
/// </summary>
6060
/// <param name="path">The input path.</param>
6161
/// <returns>A sanitized folder name.</returns>
@@ -65,7 +65,7 @@ public static string GetFolderName(string path) =>
6565
?? (IsPathSeparatorTerminated(path) ? ExtractRootFolderName(path) ?? "root" : "root");
6666

6767
/// <summary>
68-
/// Pure function: attempts to get directory name from path.
68+
/// Attempts to get directory name from path (may perform I/O).
6969
/// Returns null if invalid or current directory marker.
7070
/// </summary>
7171
private static string? TryGetDirectoryName(string path)
@@ -88,7 +88,7 @@ private static bool IsPathSeparatorTerminated(string path) =>
8888
path.EndsWith(Path.AltDirectorySeparatorChar);
8989

9090
/// <summary>
91-
/// Pure function: extracts root folder name from path.
91+
/// Extracts root folder name from path (may perform I/O).
9292
/// Removes path separators and drive colons.
9393
/// </summary>
9494
private static string? ExtractRootFolderName(string path) =>

Services/ProjectScanner.cs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public string GetProjectStructure(string projectPath, int indent = 0)
6767
}
6868

6969
/// <summary>
70-
/// Pure recursive function to generate directory structure lines.
70+
/// Recursive function to generate directory structure lines (performs I/O).
7171
/// Uses lazy evaluation via yield return.
7272
/// </summary>
7373
private IEnumerable<string> GetProjectStructureLines(string directoryPath, ScanContext context, int indent)
@@ -96,7 +96,8 @@ private IEnumerable<string> GetProjectStructureLines(string directoryPath, ScanC
9696
}
9797

9898
/// <summary>
99-
/// Pure function to get filtered and sorted directory entries.
99+
/// I/O operation: gets filtered and sorted directory entries.
100+
/// Includes console logging side effects on errors.
100101
/// </summary>
101102
private IEnumerable<string> GetFilteredEntries(string directoryPath, string rootPath)
102103
{
@@ -120,7 +121,7 @@ private IEnumerable<string> GetFilteredEntries(string directoryPath, string root
120121
}
121122

122123
/// <summary>
123-
/// Pure predicate to determine if an entry should be included.
124+
/// Predicate to determine if an entry should be included (may perform I/O).
124125
/// </summary>
125126
private bool ShouldIncludeEntry(string entryPath, string rootPath)
126127
{
@@ -172,7 +173,7 @@ public string GetFileContents(string projectPath)
172173
}
173174

174175
/// <summary>
175-
/// Pure recursive function to enumerate all files in directory tree.
176+
/// Recursive I/O operation to enumerate all files in directory tree.
176177
/// Uses lazy evaluation via yield return.
177178
/// </summary>
178179
private IEnumerable<string> EnumerateFilesRecursively(string directory, string rootPath)
@@ -200,7 +201,8 @@ private IEnumerable<string> EnumerateFilesRecursively(string directory, string r
200201
}
201202

202203
/// <summary>
203-
/// Gets filtered files from a directory (pure enumeration).
204+
/// I/O operation: gets filtered files from a directory.
205+
/// Includes console logging side effects on errors.
204206
/// </summary>
205207
private IEnumerable<string> GetFilteredFiles(string directory, string rootPath, EnumerationOptions options)
206208
{
@@ -217,7 +219,7 @@ private IEnumerable<string> GetFilteredFiles(string directory, string rootPath,
217219
}
218220

219221
/// <summary>
220-
/// Gets filtered subdirectories from a directory (pure enumeration).
222+
/// I/O operation: gets filtered subdirectories from a directory.
221223
/// </summary>
222224
private IEnumerable<string> GetFilteredDirectories(string directory, string rootPath, EnumerationOptions options)
223225
{
@@ -233,7 +235,7 @@ private IEnumerable<string> GetFilteredDirectories(string directory, string root
233235
}
234236

235237
/// <summary>
236-
/// Pure predicate to check if a file should be included.
238+
/// Predicate to check if a file should be included (may perform I/O).
237239
/// </summary>
238240
private bool ShouldIncludeFile(string filePath, string rootPath)
239241
{
@@ -249,7 +251,7 @@ private bool ShouldIncludeFile(string filePath, string rootPath)
249251
}
250252

251253
/// <summary>
252-
/// Pure predicate to check if a directory should be included.
254+
/// Predicate to check if a directory should be included (may perform I/O).
253255
/// </summary>
254256
private bool ShouldIncludeDirectory(string dirPath, string rootPath)
255257
{

Utils/FileUtilities.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ private static bool CheckFileBinaryContent(string filePath, int chunkSize, doubl
4040
}
4141

4242
/// <summary>
43-
/// Pure function: analyzes stream content to determine if binary.
43+
/// I/O operation: analyzes stream content to determine if binary.
4444
/// Checks for UTF-8 BOM first, then analyzes byte patterns.
4545
/// </summary>
4646
private static bool IsStreamBinary(FileStream stream, int chunkSize, double threshold) =>
4747
stream.Length > 0 && !HasUtf8Bom(stream) && HasBinaryContent(stream, chunkSize, threshold);
4848

4949
/// <summary>
50-
/// Pure function: checks if stream starts with UTF-8 BOM.
51-
/// Resets stream position after checking.
50+
/// I/O operation: checks if stream starts with UTF-8 BOM.
51+
/// Resets stream position after checking (side effect).
5252
/// </summary>
5353
private static bool HasUtf8Bom(FileStream stream)
5454
{
@@ -65,7 +65,7 @@ private static bool HasUtf8Bom(FileStream stream)
6565
}
6666

6767
/// <summary>
68-
/// Pure function: checks if stream content exceeds binary threshold.
68+
/// I/O operation: checks if stream content exceeds binary threshold.
6969
/// </summary>
7070
private static bool HasBinaryContent(FileStream stream, int chunkSize, double threshold)
7171
{

Utils/GitHelper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ var parent when string.IsNullOrEmpty(parent) => null,
3939
};
4040

4141
/// <summary>
42-
/// Pure predicate: checks if a directory contains a .git subdirectory.
42+
/// I/O operation: checks if a directory contains a .git subdirectory.
4343
/// </summary>
4444
private static bool HasGitDirectory(string path) =>
4545
Directory.Exists(Path.Combine(path, ".git"));

0 commit comments

Comments
 (0)