patterncsharpMinor
Idiomatic and Conventional F#
Viewed 0 times
andidiomaticconventional
Problem
As of late I have been learning F#. Today I accumulated what I have learned so far to develop my first program - a script that counts the number of lines of code in a given Visual Studio project.
The code for said program is as follows:
I have tried write both clean and idiomatic code but I still have some concerns, namely:
Of course, any additional feedb
The code for said program is as follows:
#r "System.Xml.Linq"
open System
open System.IO
open System.Xml.Linq
open System.Text.RegularExpressions
printf "Enter solution path: "
let solutionPath = Console.ReadLine()
let solutionDirPath = Path.GetDirectoryName(solutionPath)
let solutionText = File.ReadAllText(solutionPath)
let projectPaths =
Regex.Matches(solutionText, "([^\"]*csproj)")
|> Seq.cast
|> Seq.map (fun m -> Path.Combine(solutionDirPath, m.Groups.[1].Value))
let codeFilePaths =
projectPaths
|> Seq.map (fun projPath ->
let projectDirPath = Path.GetDirectoryName(projPath)
let document = XDocument.Load(projPath)
document.Descendants()
|> Seq.where (fun desc -> desc.Name.LocalName = "Compile")
|> Seq.map (fun desc -> Path.Combine(projectDirPath, desc.FirstAttribute.Value)))
|> Seq.concat
let locIsSignificant (loc : string) =
let sanitizedLoc = loc.Trim()
match sanitizedLoc with
| "" -> false
| "{" -> false
| "}" -> false
| _ -> true
let totalLoc =
codeFilePaths
|> Seq.sumBy (fun path ->
File.ReadLines(path)
|> Seq.where locIsSignificant
|> Seq.length)
printfn "Total Loc: %i" totalLoc
Console.ReadKey() |> ignoreI have tried write both clean and idiomatic code but I still have some concerns, namely:
- In this script, I introduce sequential intermediate bindings (
projectPaths->codeFilePaths->totalLoc) however, the bindinglocIsSignificantfeels like an interruption to this sequence. Have I arranged my code in an conventional way?
- Is my code as idomatic as can be? That is, could I make the code more expressive by using constructs or commands that I have not considered?
Of course, any additional feedb
Solution
Regex.Matches(solutionText, "([^\"]*csproj)")
|> Seq.cast
|> Seq.map (fun m -> Path.Combine(solutionDirPath, m.Groups.[1].Value))Why are you using groups? Since the whole match is a single group, you can remove the parentheses from the expression and then directly use
m.Value.Also, you should probably check that the name ends in
.csproj, not just csproj and that it's actually the end of the file name.let codeFilePaths =
projectPaths
|> Seq.map (fun projPath ->
let projectDirPath = Path.GetDirectoryName(projPath)
let document = XDocument.Load(projPath)
document.Descendants()
|> Seq.where (fun desc -> desc.Name.LocalName = "Compile")
|> Seq.map (fun desc -> Path.Combine(projectDirPath, desc.FirstAttribute.Value)))
|> Seq.concat-
I think the complicated
map lambda makes this hard to read, I would instead write it as a local named function. If you don't want to do that, at least make sure to indent your code properly (the first |> Seq.map and the |> Seq.concat should be on the same level).-
Instead of the
where, you can use the overload of Descendants that accepts a node name (but this means you have to use XML namespaces).-
Instead of relying on the first attribute being the right one, I would access the attribute by name.
With all these changes and John Palmer's suggestion about
Seq.collect, the code becomes:let codeFilePaths =
let codeFilesForProject projPath =
let projectDirPath = Path.GetDirectoryName(projPath)
let document = XDocument.Load(projPath)
let ns = XNamespace.Get "http://schemas.microsoft.com/developer/msbuild/2003"
document.Descendants(ns + "Compile")
|> Seq.map (fun desc -> Path.Combine(projectDirPath, desc.Attribute(XName.Get "Include").Value))
projectPaths |> Seq.collect codeFilesForProjectCode Snippets
Regex.Matches(solutionText, "([^\"]*csproj)")
|> Seq.cast<Match>
|> Seq.map (fun m -> Path.Combine(solutionDirPath, m.Groups.[1].Value))let codeFilePaths =
projectPaths
|> Seq.map (fun projPath ->
let projectDirPath = Path.GetDirectoryName(projPath)
let document = XDocument.Load(projPath)
document.Descendants()
|> Seq.where (fun desc -> desc.Name.LocalName = "Compile")
|> Seq.map (fun desc -> Path.Combine(projectDirPath, desc.FirstAttribute.Value)))
|> Seq.concatlet codeFilePaths =
let codeFilesForProject projPath =
let projectDirPath = Path.GetDirectoryName(projPath)
let document = XDocument.Load(projPath)
let ns = XNamespace.Get "http://schemas.microsoft.com/developer/msbuild/2003"
document.Descendants(ns + "Compile")
|> Seq.map (fun desc -> Path.Combine(projectDirPath, desc.Attribute(XName.Get "Include").Value))
projectPaths |> Seq.collect codeFilesForProjectContext
StackExchange Code Review Q#63420, answer score: 3
Revisions (0)
No revisions yet.