I'm learning F# and functional programming, from a background in C# and imperative/OOP. I've ported a small, one-off console app from C# to F#. The port worked (the app behaves the same way), but I'd love to get some feedback on writing more stylistic F# and thinking about code in a functional way, as this was a port of imperative code. I feel like I'm still on the second square of the diagram in this article, or imperative F# code. I don't have a good grasp of code style or any of those basic things yet either!

The console app is intended to read JSON (saved in .txt files) as HTTP POST payloads and send them to a configurable endpoint. I've tried to leave comments in my code where I have uncertainties.

namespace MyApp
// In C# I'm used to having my "using" statements inside a namespace, but before a class... Is that idiomatic in F# w/r/t modules/types?
open Newtonsoft.Json
open System.IO
open System.Linq
open RestSharp
open System.Net
open System.Reflection
module Functions =
exception FailedScript of string
// Is it correct/appropriate to restrict method parameter types like this?
// This function loads a .json file with various configurable settings
let loadEnvironmentConfig(path : string) =
let configPath = ".\\" + path + ".json"
Linq.JObject.Parse(File.ReadAllText(configPath))
// The way I've written this method it takes a tuple of strings, right?
// Is it idiomatic to 'bundle' method params in this way?
// This function is intended to navigate up the directory hierarchy until it finds one with the provided name
let rec findDirectoryFrom(path : string, name : string) =
// Good to invoke the ctor without parens?
let parent = DirectoryInfo path
// Is there a more F#-y way to perform this linq evaluation?
let possibleMatch = parent.GetDirectories(name).FirstOrDefault()
if possibleMatch <> null then possibleMatch else findDirectoryFrom(parent.Parent.FullName, name)
// This method is intended to find all the .txt files in a directory, and the directory's child directories.
// Then it deserializes the JSON within the file.
let rec getScriptsFromDirectory(directory : DirectoryInfo) =
let scripts = directory.EnumerateFiles("*.txt").Select(fun f -> JsonConvert.DeserializeObject(File.ReadAllText(f.FullName)))
let children = directory.GetDirectories()
seq {
yield! scripts
// Is there a more idiomatic way to express this than the for-do pattern?
for directory in children do yield! getScriptsFromDirectory(directory)
}
// I tried to write this function without typing the params in a tuple.
// It feels very unnatural to me coming from C#, especially the need to cast the 'client' param on the first line of the function!
// F# doesn't support duck-typing, so you have to perform the cast... Seems better to explicitly type the params in this case, am I correct?
let executeScript script client =
let client = client :> RestClient
let request = RestRequest(Method.POST)
request.RequestFormat <- DataFormat.Json
// This ignore feels like a bug to me, but it does work. AddParameter clearly works via side-effect, what's a better way to express this?
request.AddParameter("application/json", script, ParameterType.RequestBody) |> ignore
let response = client.Execute(request)
if response.StatusCode = HttpStatusCode.Conflict || response.StatusCode = HttpStatusCode.Created then response.StatusCode else raise(FailedScript(response.ErrorMessage))
// This function is supposed to be the main execution point
let executeScripts(config : Linq.JObject) =
// This feels very procedural...
let executingPath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)
let directory = findDirectoryFrom(executingPath, "scripts")
let scripts = getScriptsFromDirectory(directory)
let endpoint = config.GetValue("DeploymentApiEndpoint").ToString()
let client = RestClient(endpoint)
// It feels very strange to me that I have to cast an implementation back to its interface before I can pass it into a method that requires said interface. Am I doing something wrong?
let authenticator = HttpBasicAuthenticator(config.GetValue("UserName").ToString(), config.GetValue("Password").ToString()) :> IAuthenticator
client.Authenticator = authenticator |> ignore
for script in scripts do
printf "%A" (executeScript script client)

Annotations are normally required for OO style code. When you do x.Length the compiler has no idea what x is, so you have to annotate with string or whatever.

Pattern matching

It's unidiomatic to use FirstOrDefault and then check for null, especially as F# types can't be null!
Instead, convert to a list and use pattern matching. Unlike FirstOrDefault, you can never accidentally forget to handle the empty list case.

When you see messages about "should be a unit", don't just use ignore without understanding the error.
In this case, the code below is probably buggy. You are comparing the two values for equality rather than doing assignment!

client.Authenticator = authenticator |> ignore

I think what you wanted was to assign the value like this:

client.Authenticator <- authenticator

Setting properties in the constructor

F# allows you to pass properties in the constructor, which eliminates a lot of assignment ugliness. So you can write something like this:

Now we have pushed the problem up a level! What happens when findDirectoryFrom fails?

In this code, I'm returning a None, so now the outer code has to change to deal with the edge case.

I leave that as an exercise to the reader (hint - use Option.map in the executeScripts pipeline)
but you can see that type-safety is forcing your code to deal with cases that you could too easily ignore in OO.