Est-ce que cette utilisation correcte de la correspondance de motif et des motifs actifs est correcte?

Question

Je suis nouveau sur F # et fonctionnel et je travaille sur du code d'analyse HTML. Je souhaite supprimer d'un document HTML les éléments correspondant à certains critères. Ici, j'ai une séquence d'objets (HtmlNodes) et je veux les retirer du document.

Est-ce une façon idiomatique d'utiliser la correspondance de formes? De même que HtmlNode.Remove() a un effet secondaire sur l'objet HtmlDocument d'origine, existe-t-il un moyen particulier de structurer le code pour rendre l'effet secondaire évident ou comment cela devrait-il être géré. Vous pouvez être aussi pédant que vous le souhaitez avec le code.

open HtmlAgilityPack 

let removeNodes (node : HtmlNode) = 

    let (|HiddenNodeCount|) (n : HtmlNode) = 
     match n.SelectNodes("*[@style[contains(.,'visibility:hidden')]]") with 
     | null -> 0 
     | _ as x -> Seq.length x 

    match node with 
     | x when x.Name.ToLower() = "script" -> node.Remove() 
     | x when x.NodeType = HtmlNodeType.Comment -> node.Remove() 
     | HiddenNodeCount x when x > 0 -> node.Remove() 
     | _ ->() 

let html = "some long messy html code would be here" 
let dom = new HtmlDocument(OptionAutoCloseOnEnd=true) 
dom.LoadHtml(html) 

let nodes = dom.DocumentNode.DescendantNodes() 
nodes |> Seq.toArray |> Array.iter removeNodes 

Answer 1

Personnellement, je préfère if elif else sur pattern matching lorsque vous ne disposez pas d'une structure de données à se décomposer (il est un peu moins à taper, et peut également servir à différencier entre le moment où une structure est décomposée par rapport à des tests de cas le plus simple) .

Il y a des choses étranges dans votre code. Le motif actif n'est pas très utile ici pour deux raisons: d'abord, sa portée est limitée à removeNodes afin qu'il ne soit utilisé qu'une seule fois. J'aborderai les deuxièmes problèmes plus tard, mais je vais d'abord montrer comment j'écrirais ceci en éliminant le pattern actif et, pour moi du moins, en rendant les effets secondaires plus évidents (en séparant le code qui teste si un nœud doit être retiré du code qui fait la suppression):

let shouldRemoveNode (node : HtmlNode) = 
    if node.Name.ToLower() = "script" then true 
    elif node.NodeType = HtmlNodeType.Comment then true 
    else match node.SelectNodes("*[@style[contains(.,'visibility:hidden')]]") with 
     | null -> false 
     | x -> Seq.length x > 0 

let removeNode (node: HtmlNode) = 
    if shouldRemoveNode(node) then node.Remove() else() 

Avis J'utilise une correspondance de motif dans la visibilité requête cachée depuis que je ne reçois de match contre nul et se lier à x autrement (plutôt que de se lier à x, et puis test x avec if else). La deuxième chose étrange avec votre Active Pattern est que vous l'utilisez pour convertir un nœud à un int, mais la longueur que vous obtenez n'est pas immédiatement utile (vous devez encore effectuer un test contre cela). Alors que l'utilisation plus puissante d'un pattern actif ici serait de découper des nœuds en différents types (en supposant que ce n'est pas ad-hoc, ce qui était le premier point). Donc, vous pourriez avoir:

//expand to encompass several other kinds of nodes 
let (|Script|Comment|Hidden|Other|) (node : HtmlNode) = 
    if node.Name.ToLower() = "script" then Script 
    elif node.NodeType = HtmlNodeType.Comment then Comment 
    else match node.SelectNodes("*[@style[contains(.,'visibility:hidden')]]") with 
     | null -> Other 
     | x -> if Seq.length x > 0 then Hidden 
       else Other 

let removeNode (node: HtmlNode) = 
    match node with 
    | Script | Comment | Hidden -> node.Remove() 
    | Other ->() 

Edit:

@Pascal a fait l'observation dans les commentaires que shouldRemoveNode peut encore être condensé dans une grande expression booléenne:

let shouldRemoveNode (node : HtmlNode) = 
    node.Name.ToLower() = "script" || 
    node.NodeType = HtmlNodeType.Comment || 
    match node.SelectNodes("*[@style[contains(.,'visibility:hidden')]]") with 
    | null -> false 
    | x -> Seq.length x > 0 

Answer 2

Il n'est pas clair pour moi que cela soit meilleur que d'utiliser des fonctions et si-alors-autrement, par ex.

let HiddenNodeCount (n : HtmlNode) = 
    match n.SelectNodes("*[@style[contains(.,'visibility:hidden')]]") with 
    | null -> 0 
    | x -> Seq.length x 

if node.Name.ToLower() = "script" then 
    node.Remove() 
elif node.NodeType = HtmlNodeType.Comment then 
    node.Remove() 
elif HiddenNodeCount node > 0 then 
    node.Remove()