4
\$\begingroup\$

Requirements

Note: the requirements are invented by me for practicing functional programming.

Functional Requirements

Given a directory, all files below the directory (and its sub directories) should be filtered by one of the available filter conditions and then printed to the console.

The filter condition can be

a) a filter function based on a FileInfo object

  • If the file passes the filter, the full path should be printed to console

b) a Regex that tries to match the content of the file.

  • Binary files should be always filtered out
  • If the file passes the filter, the full path should be printed to the console
  • For each matching line, the line number and the full line should be printed to the console

Technical Requirements

  • the solution should be as functional as possible
  • the solution should work without mutable state
  • For performance reasons, the whole program should work lazy (we don't want to create a large list of all files and it's state and finally working on that data structure)

Solution

Usage

[<EntryPoint>]
let main argv = 

    let path = @"C:\Temp\CommandLineFSharp"

    // regex file content fiter
    let regex = Regex("Regex", RegexOptions.Compiled ||| RegexOptions.IgnoreCase)
    let title = "All files that match the regex 'Regex'"
    let allFilesContainingError = FileContentRegexFilter(title, regex)
    findIn path allFilesContainingError

    // FileInfo filter
    let title = "All files whose name start with 'A'"
    let allFilesStartingWithA = FileInfoFilter(title, fun fi -> fi.Name.StartsWith("a", StringComparison.InvariantCultureIgnoreCase))
    findIn path allFilesStartingWithA

    Console.ReadLine() |> ignore
    0

Output

 /*--------------------------------------------------------------------------------
 All files that match the regex 'Regex'
--------------------------------------------------------------------------------
C:\Temp\CommandLineFSharp\FileSystem.fs
   [line: 18]         | FileContentRegexFilter of String * Regex
   [line: 46]     let private getMatchingLines (r:Regex) (file:FileInfo) =
   [line: 59]             | FileContentRegexFilter (title, regex) ->
   [line: 60]                 let matchingLines = file |> (getMatchingLines regex)
   [line: 76]             | FileInfoFilter(title, _) | FileContentRegexFilter(title, _) ->
C:\Temp\CommandLineFSharp\Program.fs
   [line: 12]     // regex file content fiter
   [line: 13]     let regex = Regex("Regex", RegexOptions.Compiled ||| RegexOptions.IgnoreCase)
   [line: 14]     let title = "All files that match the regex 'Regex'"
   [line: 15]     let allFilesContainingError = FileContentRegexFilter(title, regex)
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
 All files whose name start with 'A'
--------------------------------------------------------------------------------
C:\Temp\CommandLineFSharp\App.config
C:\Temp\CommandLineFSharp\AssemblyInfo.fs
-------------------------------------------------------------------------------- */

Implementation

// Configuration
let private lineLength = 80
let private lineChar = '-'

// Types
type private FileSystemItem =
    | File of FileInfo
    | Directory of DirectoryInfo * FileSystemItem seq

type FileItemFilter =
    | FileInfoFilter of string * (FileInfo -> bool)
    | FileContentRegexFilter of String * Regex

// Private Functions 
let private isTextFile (file:FileInfo) =
    use sr = new StreamReader (file.OpenRead())
    let isControlChar ch = Char.IsControl(ch) && ch <> '\r' && ch <> '\n'
    let rec processReader (reader:StreamReader) =
        match reader with
            | r when r.EndOfStream -> true
            | r when r.Read() |> char |> isControlChar -> false
            | _ -> processReader reader
    processReader sr

let private enumerateLines (file:FileInfo) = seq {
    use sr = new StreamReader (file.OpenRead())
    while not sr.EndOfStream do
        yield sr.ReadLine ()
    }

let private printMatchingLine m = 
    printfn "   [line: %d] %s" (snd m) (fst m)

let private printFile (file:FileInfo) = 
    printfn "%s" file.FullName

let private printLine () = 
    printfn "%s" (String(lineChar, lineLength))

let private getMatchingLines (r:Regex) (file:FileInfo) = 
        if (file |> isTextFile) then
            file 
            |> enumerateLines 
            |> Seq.mapi (fun idx line -> (line, idx+1))
            |> Seq.filter (fst >> r.IsMatch)    
        else
            Seq.empty

let private processFilter filter file =
    match filter with
        | FileInfoFilter (title, passFilter) -> 
            if file |> passFilter then printFile file                      
        | FileContentRegexFilter (title, regex) -> 
            let matchingLines = file |> (getMatchingLines regex)
            if matchingLines |> (not << Seq.isEmpty) then
                file |> printFile
                matchingLines |> Seq.iter printMatchingLine

let rec private createDirectory (directoryInfo:DirectoryInfo) = 
    let dir = directoryInfo
    let subs = seq {
        for d in dir.EnumerateDirectories() |> Seq.map createDirectory  do yield d
        for f in dir.EnumerateFiles()       |> Seq.map File             do yield f
    }
    Directory(dir, subs)       

// Public Functions
let findIn directoryPath filter =
    match filter with
        | FileInfoFilter(title, _) | FileContentRegexFilter(title, _) ->
            printLine ()
            printfn @" %s" title
            printLine ()                

    let rec findInternal fileSystemItem =
        match fileSystemItem with
            | File(fi) -> processFilter filter fi
            | Directory(di, subs) -> subs |> Seq.iter findInternal 

    findInternal (directoryPath |> (DirectoryInfo >> createDirectory))

    printLine ()
    printfn ""

Questions

Of course, every feedback is welcome!

Readability:

I've tried to be as descriptive as possible. However, F# provides lots of ways of doing the same (e.g. (func val) or (val |> func)). Any suggestion for further improvements / are there any fragments that are hard to understand?

Simplicity

Is it possible to simplify some of the code fragments?

Design/Extensibility

That is the point I am most interested in. Actually, the code works fine for the functional requirements mentioned above. But as far as new (even small) requirements come up, it feels that large parts of the program must be rewritten.

For example:

  • New Filter Condition

A new filter FileContentRegexStrFilter of String * String should be added. The filter provides not the Regex object but a string representing the regex pattern. Internally, a single (compiled) Regex object should be created and used during the whole execution.

The Problem here is, that it is not possible to create a shared object during pattern matching that can be reused.

  • Add Search Summary

At the end of the search, the number of all searched directories, the total number of files as well as the number of files that passed the filter should be displayed.

My brain provides just a mutable counter as pattern for that kind of problem - actually, I have absolutely no idea how to realize that without mutable state....

Is there a way (or something like best practices similar to SOLID for OOP) for designing / organizing functional code to become more extensible?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I meant to answer this a while back, but I completely forgot. At any rate, let's talk about a couple things.

First, try not to use a pattern match if there's no pattern. In this case you have:

    match reader with
        | r when r.EndOfStream -> true
        | r when r.Read() |> char |> isControlChar -> false
        | _ -> processReader reader

When

if reader.EndOfStream = true then true
elif reader.Read() |> char |> isControlChar = true then false
else processReader reader

Will do just fine, and in fact may be easier to read for some (YMMV).


This next block looks like it violates SRP a lot:

let findIn directoryPath filter =
    match filter with
        | FileInfoFilter(title, _) | FileContentRegexFilter(title, _) ->
            printLine ()
            printfn @" %s" title
            printLine ()                

    let rec findInternal fileSystemItem =
        match fileSystemItem with
            | File(fi) -> processFilter filter fi
            | Directory(di, subs) -> subs |> Seq.iter findInternal 

    findInternal (directoryPath |> (DirectoryInfo >> createDirectory))

    printLine ()
    printfn ""

Why does a findIn have to print everything it found? Why can't it just return a sequence of the results?


Let's talk about the specific concerns you have.

I've tried to be as descriptive as possible. However, F# provides lots of ways of doing the same (e.g. (func val) or (val |> func)). Any suggestion for further improvements / are there any fragments that are hard to understand?

Generally, it doesn't matter which style you use, though using the pipe-right (or pipe-next) operator has a better readability. Consider the following snippet:

let value2 =
    values
    |> Array.toList
    |> List.map (fun v -> Math.Sin(float v))
    |> List.choose (fun v -> if v >= 0.0 then Some v else None)
    |> List.rev
    |> List.takeWhile (fun v -> v > 0.05)
    |> List.fold (fun v acc -> acc + v) 0.0

Versus:

let value2 =
    List.fold (fun v acc -> acc + v) 0.0
        (List.takeWhile (fun v -> v > 0.05)
        (List.rev
        (List.choose (fun v -> if v >= 0.0 then Some v else None)
        (List.map (fun v -> Math.Sin(float v))
        (Array.toList values)))))

I shouldn't have to tell you which one I would prefer. Just look at all those parenthesis.

F# also contains a double-pipe-right and triple-pipe-right operator, which will pipe a tuple to the function. (I'm not sure if you were already aware of that or not.) So you can write something like:

let append s1 s2 = s1 + "." + s2
let result = ("abc", "def") ||> append

And regarding readability, keep your indentation consistent:

let private getMatchingLines (r:Regex) (file:FileInfo) = 
        if (file |> isTextFile) then
            file 
            |> enumerateLines 
            |> Seq.mapi (fun idx line -> (line, idx+1))
            |> Seq.filter (fst >> r.IsMatch)    
        else
            Seq.empty

Let's bring that back a few spaces:

let private getMatchingLines (r:Regex) (file:FileInfo) = 
    if (file |> isTextFile) then
        file 
        |> enumerateLines 
        |> Seq.mapi (fun idx line -> (line, idx+1))
        |> Seq.filter (fst >> r.IsMatch)    
    else
        Seq.empty

Is it possible to simplify some of the code fragments?

You didn't define 'simplify', but let's consider fewer LoC. You don't really have many places (without a major structural rewrite) that you can reduce LoC. Your code is currently good and it's also concise. Everything (almost) follows SRP well, and it's easy to reason about for the most part.

You could make things a bit easier to reason about by considering places where you use multiple compositions in opposite directions, such as matchingLines |> (not << Seq.isEmpty). This isn't particularly hard to reason about, but it's not immediately obvious. We could consider a rewrite that makes it a bit more obvious: matchingLines |> Seq.isEmpty |> not. Now that doesn't read to a human as simply, I can see why you wrote it the way you did, but if we follow the mantra of imagining the next maintainer is a psychopath with a sword who knows where you live, and has a very low breaking point, this probably isn't the best way to write it.


That is the point I am most interested in. Actually, the code works fine for the functional requirements mentioned above. But as far as new (even small) requirements come up, it feels that large parts of the program must be rewritten.

...

Is there a way (or something like best practices similar to SOLID for OOP) for designing / organizing functional code to become more extendable?

You've discovered one of the major downfalls of FP: it's built to just work, and it's generally not as extensible as one would like. One of the major disadvantages of immutability and designing for it is that to add something that, in a language such as C# would be so simple, you usually end up making major changes.

However, this doesn't have to be the case. You could reduce the impact with memoization, function composition and a basic object / type / class to contain all your work. Of course, both of the modifications you mention are not small modifications, so there is a little work involved.


  • New Filter Condition

A new filter FileContentRegexStrFilter of String * String should be added. The filter provides not the Regex object but a string representing the regex pattern. Internally, a single (compiled) Regex object should be created and used during the whole execution.

The Problem here is, that it is not possible to create a shared object during pattern matching that can be reused.

You're thinking about this the wrong way. With a functional language everything can be considered part of a function. Instead of creating a 'shared object', simply map the filter to a function. You're adding a whole new level of features, so this modification will become quite large.

The first modification:

type FileItemFilter =
    | FileInfoFilter of string * (FileInfo -> bool)
    | FileContentRegexFilter of String * Regex
    | FileContentRegexStrFilter of String * String

The part I think you're getting caught up on is right here:

match filter with
    | FileInfoFilter (title, passFilter) -> 
        if file |> passFilter then printFile file                      
    | FileContentRegexFilter (title, regex) -> 
        let matchingLines = file |> (getMatchingLines regex)
        if matchingLines |> (not << Seq.isEmpty) then
            file |> printFile
            matchingLines |> Seq.iter printMatchingLine

Well, we don't care about that. We really don't. We're going to convert the FileContentRegexStrFilter to a FileContentRegexFilter before it even gets there.

We're going to work right here:

let findIn directoryPath filter =
    match filter with
        | FileInfoFilter(title, _) | FileContentRegexFilter(title, _) ->
            printLine ()
            printfn @" %s" title
            printLine ()                

    let rec findInternal fileSystemItem =
        match fileSystemItem with
            | File(fi) -> processFilter filter fi
            | Directory(di, subs) -> subs |> Seq.iter findInternal 

In fact, we're just going to add a few LoC to fix it:

let filterConvert filter =
    match filter with
    | FileContentRegexStrFilter(title, regex) -> FileContentRegexFilter(title, regex)
    | _ -> filter

let findIn directoryPath inFilter =
    let filter = filterConvert inFilter
    ...

The rest remains the same.

Now by doing it this way, we can add more filter conversions and only change two locations: add a type, add a match clause.


  • Add Search Summary

At the end of the search, the number of all searched directories, the total number of files as well as the number of files that passed the filter should be displayed.

My brain provides just a mutable counter as pattern for that kind of problem - actually, I have absolutely no idea how to realize that without mutable state....

Is there a way (or something like best practices similar to SOLID for OOP) for designing / organizing functional code to become more extensible?

Once again, you fall into the trap of going from an OOP / procedural structure to a functional one. To program in a functional language you have to rewrite how you think. You can't think about objects and state, you have to think about 'what is the relation from a -> b?' F# is all about what input results in what output, we have to think that way.

The first thing we need to do is indicate if we matched a file or not in processFilter:

let private processFilter filter file =
    match filter with
    | FileInfoFilter (title, passFilter) -> 
        if file |> passFilter then
            printFile file
            true
        else false
    | FileContentRegexFilter (title, regex) -> 
        let matchingLines = file |> (getMatchingLines regex)
        if matchingLines |> (not << Seq.isEmpty) then
            file |> printFile
            matchingLines |> Seq.iter printMatchingLine
            true
        else false

Easy enough.

Next, we're going to use Seq.fold instead of Seq.iter, and a tuple to store our information.

let rec findInternal fileSystemItem =
    match fileSystemItem with
    | File(fi) ->
        if processFilter filter fi = true then (1, 0, 1)
        else (1, 0, 0)
    | Directory(di, subs) ->
        subs |> Seq.fold (fun acc x ->
        let filesSeen, dirsSeen, matched = findInternal x
        let accSeen, accDirs, accMatched = acc
        (accSeen + filesSeen, accDirs + dirsSeen, accMatched + matched)) (0, 1, 0)

Finally, we'll print:

let seen, dirs, matched = findInternal (directoryPath |> (DirectoryInfo >> createDirectory))
printfn "Files Seen: %i, Directories Searched: %i, Files Found: %i" seen dirs matched

That's not so bad.


Hopefully you learned something from this, and I hope that it helps you think more functionally in the future. Overall your code isn't bad, and taking some of these tips into account (and helping force yourself to think functionally) can help you become a far better F# programmer.

\$\endgroup\$
2

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.