My first F# program...

Question

The idea was to create/populate a perfmon counter with the # of files in a directory. Apparently FileNet (a program) uses a given directory as a queue, and gets hung up sometimes. Hence the need to monitor the # of files that are stacking up...

Now most of you will probably LOL your asses off at my spider web of "if/then" statements, but I liked writing this enough to want to get better and understand where I'm being inefficient. I'm hoping someone out there will give me a few pointers :)

Fully working source (I'm compiling for .NET 2.0 using fsc) http://cl1p.net/fileNetCount

(thanks to @Dan for introducing me to this nifty-non-bracketed language)

Answer 1

You should assign the arguments to some named values as soon as possible, so it's possible to know what they mean without looking back at the usage message.

Also, you should break this up into methods. The logic to check if input is OK for "create", "scan" or "remove" should live in 3 separate methods. Likewise, the main code itself should have methods as well. That way, the overall logic of the program would be clear at a glance when looking at the initially-executed code.

These suggestions aren't necessarily functional, but they'd definitely make the code more readable and meaningful.

Answer 2

It's an F# program written in imperative style. Which is fine, except that you're missing out on all of the goodness that is functional programming.

I would start out by trying to write something simple like a Fibonnaci sequence or a program that will compute a factorial. Write it in functional style, using recursion.

Answer 3

My initial impression is that there is nothing in your application that would make F# a compelling choice. That said, here are a few examples of ways you could use F# features to simplify your logic:

// Bring the namespace into scope, like using in C#
open System.Diagnostics

// Methods with descriptive parameter names are good
let Create counter_category counter_name =
    match PerformanceCounterCategory.Exists(counter_category) with
    // Using a guard eliminates a second level of if/then or matching
    | true when PerformanceCounterCategory.CounterExists(counter_name, counter_category)
           -> printfn "Performance counter %A already exists" counter_name
    | true -> printfn "Performance counter category %A already exists" counter_category
    // _ is effectively an "else" clause, or a switch's default
    | _    -> let CounterDatas = new CounterCreationDataCollection()
              let cdCounter1 = new CounterCreationData()
              cdCounter1.CounterName <- counter_name
              cdCounter1.CounterType <- PerformanceCounterType.NumberOfItems64
              CounterDatas.Add(cdCounter1) |> ignore
              printf "Creating category and counter: "
              PerformanceCounterCategory.Create(counter_category, "", PerformanceCounterCategoryType.SingleInstance, CounterDatas) |> ignore
              printfn "Success."

// Fake implementations, left as an exercise
let Scan counter_category counter_name directory =
    printfn "%s %s %s %s" "Scan" counter_category counter_name directory

let Remove counter_category counter_name =
    printfn "%s %s %s" "Remove" counter_category counter_name

// The only place we need to specify a type
let ParseArgs (args : string array) =
    let app = args.[0]
    match args.Length with
    | 1 -> printfn "Usage: %A [create|scan|remove]" app
    // By pairing up the op and number of arguments...
    | n -> match args.[1],n with
           // We can easily handle good cases...
           | "create",4 -> Create args.[2] args.[3]
           | "scan",5   -> Scan args.[2] args.[3] args.[4]
           | "remove",4 -> Remove args.[2] args.[3]
           // And bad cases...
           | "create",_ -> printfn "Usage: %A create <CounterCategory> <CounterName>" app
           | "scan",_   -> printfn "Usage: %A scan <CounterCategory> <CounterName> <Directory>" app
           | "remove",_ -> printfn "Usage: %A remove <CounterCategory> <CounterName>" app
           | op,_       -> printfn "Invalid operation: %A" op

// Sys.argv is just for compatibility
ParseArgs (System.Environment.GetCommandLineArgs())

Answer 4

You're doing a lot of conditional logic. Try using some match statements. It's not necessarily functional but it will improve readability and steer you in the right direction.

Adventures in F# - F# 101 Part 5 (Pattern Matching)
Match Expressions (F#) MSDN