4
\$\begingroup\$

I want to build a small console program, that for given directory, will print (for now) all files inside that tree.

Here is an F# script that I came up with:

open System.IO

let getDirObjects (dir :DirectoryInfo) =
    let dirs = dir.GetDirectories()
    let files = dir.GetFiles()
    dirs,files

let rec traverse allFiles dir =
    let dirs,files = getDirObjects dir
    let allFiles' = Seq.append allFiles files
    let allFiles'' = dirs |> Seq.map (traverse allFiles') |> Seq.concat
    Seq.append allFiles' allFiles''

let printFileInfo (fileInfo:FileInfo) =
    printfn "%A" fileInfo.FullName

let printAllFilesFromDir path =
    let dirInfo = DirectoryInfo path
    let allFiles = traverse Seq.empty dirInfo
    allFiles |> Seq.iter printFileInfo

Changelog:

#1 changed List to Seq to be more generic, no need to convert Arrays returned by GetDirectories and GetFiles to List

I've been reading about F# for past 2 weeks or so. This is actually first toy program that I wrote :)

One thing I don't like is getDirObjects function, that returns a tuple DirectoryInfo * FileInfo. Maybe this could be done in a more fancy way, with pattern matching or sth?

Update

Here is much shorter version, whith same output. It's get the job done, but my first intention was to do it in an F#, functional way. Please comment on the code above.

open System.IO

let printAllFilesFromDir path =
    Directory.EnumerateFiles(path,"*",SearchOption.AllDirectories)
        |> Seq.iter (printfn "%A")

printAllFilesFromDir "D:\_FTP"
\$\endgroup\$
  • 1
    \$\begingroup\$ why not just Directory.EnumerateDirectories(path,"*",SearchOption.AllDirectories)? \$\endgroup\$ – user110704 Jul 14 '16 at 18:53
  • \$\begingroup\$ okay, this is not so important. Why you want use List instead of Array? \$\endgroup\$ – user110704 Jul 14 '16 at 18:59
  • \$\begingroup\$ Just a habit from C#, I think. Even better, this could be done with Directory.EnumerateFiles \$\endgroup\$ – wojciech_rak Jul 14 '16 at 19:02
  • \$\begingroup\$ you mean List from Collections.Generic? \$\endgroup\$ – user110704 Jul 14 '16 at 19:03
  • 1
    \$\begingroup\$ really? Directory.EnumerateFiles returns the fullname, if I remember correctly \$\endgroup\$ – user110704 Jul 14 '16 at 19:11
2
\$\begingroup\$
let allFiles' = Seq.append allFiles files
let allFiles'' = dirs |> Seq.map (traverse allFiles') |> Seq.concat

Just because you can use ' to differentiate variables does not mean that you should. Unless it's very clear what the different variables mean, you should use separate name for each of them.


dirs |> Seq.map (traverse allFiles') |> Seq.concat

You can simplify Seq.map followed by Seq.concat to just Seq.collect:

dirs |> Seq.collect (traverse allFiles')

let rec traverse allFiles dir =
    let dirs,files = getDirObjects dir
    let allFiles' = Seq.append allFiles files
    let allFiles'' = dirs |> Seq.map (traverse allFiles') |> Seq.concat
    Seq.append allFiles' allFiles''

This code is wrong. You pass files from the parent directory to each child directory and then return the parent directory files from each child directory and concat them.

For example, if you had two subdirectories, dir1 and dir2 and two files, file-in-root and dir1/file-in-dir1, then your code outputs:

file-in-root
file-in-root
file-in-root
dir2/file-in-dir2

Notice how file-in-root is output three times: once for the root directory, and also once for each subdirectory.

The solution here is not to pass allFiles to traverse:

let rec traverse dir =
    let dirs,files = getDirObjects dir
    let filesInDirs = dirs |> Seq.collect traverse
    Seq.append files filesInDirs

let dirs = dir.GetDirectories()
let files = dir.GetFiles()

Since you're using lazy seqs extensively, it would make sense to make dirs and files lazy seqs too, by using seq-returning Enumerate* methods instead of array-returning Get* methods. This way, if you enumerate the resulting seq only partially, it's going to be more efficient.

let dirs = dir.EnumerateDirectories()
let files = dir.EnumerateFiles()
\$\endgroup\$
  • 2
    \$\begingroup\$ Maybe traverse should be written as a sequence expression ; less "noise", no ? \$\endgroup\$ – Sehnsucht Jul 19 '16 at 19:24
  • \$\begingroup\$ @Sehnsucht You mean something like seq { let dirs,files = getDirObjects dir in yield! files; for dir in dirs do yield! traverse dir }? Yeah, that does look slightly better. \$\endgroup\$ – svick Jul 19 '16 at 22:39
  • 1
    \$\begingroup\$ yeah something like that, accumalting a Seq using Seq.append felt unusual ^^ ; we can even get rid of getDirObjects : seq { yield! dir.EnumerateFiles (); for dir in dir.EnumerateDirectories () do yield! traverse dir } that way the code express directly the intent : yield each file in dir and then for each subdir traverse it \$\endgroup\$ – Sehnsucht Jul 20 '16 at 6:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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