I'm new to F#, so I've ported an old Python script I have to F#. It scans a directory for files named something like Game.Of.Thrones.S01E02.xvidblahblah.mp4 or The_Wire_s03e04.somethingelse.avi, and renames them to a nice form like TheWire_s03e04_Hamsterdam.avi (where Hamsterdam is the episode's title, found from a web api). I'd welcome any advice on how to make it more idiomatic or efficient.

open System.Text.RegularExpressions
open System
open System.IO
open FSharp.Data

let RegexPattern pattern input =
    let m = Regex.Match(input, pattern)
    if m.Success then Some(List.tail [for g in m.Groups -> g.Value])
    else None

let (|ShortShowCode|_|) input = RegexPattern @"(.*)[sS](\d{1,2})[^\d]?[eE](\d{1,2})" input
let (|LongShowCode|_|) input = RegexPattern @"(.*)Season ?(\d{1,2}) ?Episode ?(\d{1,2})" input

let uncamelcase phrase = Regex.Replace(phrase, @"(?=[A-Z0-9])", " ")
let compressSpaces phrase = Regex.Replace(phrase, @"[^a-zA-Z0-9]+", " ")
let trim (phrase:string) = phrase.Trim()

let prettify title = title |> uncamelcase |> compressSpaces |> trim

let compress title = Regex.Replace(title, @"\W", "")

let apiUrl showName = sprintf "http://api.tvmaze.com/singlesearch/shows?q=%s&embed=episodes" (Uri.EscapeUriString(showName))
type Shows = JsonProvider<"http://api.tvmaze.com/singlesearch/shows?q=The%20Sopranos&embed=episodes">

let extension filePath = Path.GetExtension(filePath).ToLower()
let fileNameOnly filePath = Path.GetFileName(filePath)

let rec walk dir =
    seq { yield! Directory.EnumerateFiles(dir, "*")
          for subDir in Directory.EnumerateDirectories(dir) do
              yield! walk subDir }

type VideoFileInfo (showName: string, season: int, number: int, fileName: string) =
    member this.ShowName = showName
    member this.Season = season
    member this.Number = number 
    member this.FileName = fileName
    member this.Extension = extension fileName

let videoInfo vidFileName =
    match vidFileName with
    | ShortShowCode [showName; season; number]
    | LongShowCode [showName; season; number] 
        -> Some <| new VideoFileInfo(prettify showName, season = Int32.Parse season, number = Int32.Parse number, fileName = vidFileName)
    | _ -> None

let episodeMatch (toMatch: VideoFileInfo) (episode: Shows.Episode) = 
    (episode.Season = toMatch.Season && episode.Number = toMatch.Number)

let episodeInfo (epData: VideoFileInfo) =
    let show = epData.ShowName |> apiUrl |> Shows.Load
    show.Embedded.Episodes |> Seq.find (episodeMatch epData)

let titleImprovement vidInfo =
    let epInfo = episodeInfo vidInfo
    let newName = sprintf "%s_s%02ie%02i_%s%s" (compress vidInfo.ShowName) epInfo.Season epInfo.Number (compress epInfo.Name) vidInfo.Extension
    if vidInfo.FileName = newName then None 
    else Some(vidInfo.FileName, newName)

let renameVideo (dir: string) (oldTitle: string) (newTitle: string) =
    File.Move(dir + oldTitle, dir + newTitle)
    // Prints the command to reverse this rename!
    sprintf "rename \"%s%s\" \"%s%s\"" dir newTitle dir oldTitle

let printeach list = list |> Seq.iter (printfn "%A")

let testfiledir = "C:\\Dummy\\vids\\"
let testfilenames = ["The.Sopranos.Season 5 Episode 6.mp4"; "I am not an episode.txt"; "GameOfThrones_s01e02_TheKingsroad.m4v"; "TheWire.S03E04.avi"]
let createTestFiles dir filenames = filenames |> Seq.iter (fun filename -> File.WriteAllText(dir + filename, ""))

let main argv = 
    createTestFiles testfiledir testfilenames
    walk testfiledir 
    |> Seq.map fileNameOnly 
    |> Seq.choose videoInfo 
    |> Seq.choose titleImprovement 
    |> Seq.map (fun (oldTitle, newTitle) -> renameVideo testfiledir oldTitle newTitle)
    |> printeach

Looks good; there's nothing I'd really do differently. Good structure, no overly imperative code, appropriate use of pipes (including not piping for calling a single function with a single argument, which many people do, but I think reduces readability), pattern matching, even Active Patterns. Nice use of a type provider with a web data source too.

Two very minor things:

  • You don't need new when calling a constructor; the compiler only recommends it when creating an instance implementing IDisposable. (I'm very happy about not needing new, because I find it terribly annoying and redundant in C#.)
  • I've recently started either putting if-then-else on a single line if short enough, or giving the then part its own line as well because that better represents the structure of the construct.

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.