HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Recursively traverse directory tree and print all files

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
directoryalltraverserecursivelyprintfilesandtree

Problem

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"

Solution

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()

Code Snippets

let allFiles' = Seq.append allFiles files
let allFiles'' = dirs |> Seq.map (traverse allFiles') |> Seq.concat
dirs |> Seq.map (traverse allFiles') |> Seq.concat
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''
let rec traverse dir =
    let dirs,files = getDirObjects dir
    let filesInDirs = dirs |> Seq.collect traverse
    Seq.append files filesInDirs

Context

StackExchange Code Review Q#134889, answer score: 2

Revisions (0)

No revisions yet.