主页 / 问题 / 611266
Accepted
Ev_Hyper
Asked:2020-01-06 00:20:25 +0000 UTC2020-01-06 00:20:25 +0000 UTC

函数重构

  • 772

需要从某些站点提取信息。为了解析 HTML，我使用F# Data: HTML Parser（HTML Type Provider不幸的是，在这种情况下不适用）。

像这样实现它：

let getNextLink (document : HtmlDocument) = 
    document.Descendants "a"
    |> Seq.choose
        (fun node ->
            match node.TryGetAttribute "href" with
            |Some href when node.InnerText().Trim() = "ключевое слово" -> 
               href.Value() |> Some
            |_ -> None)
    |> Seq.tryHead

let getAllValues start  = 
    let rec loop (pages : string) = seq {
        let result = HtmlDocument.Load pages
        yield 
            result.Descendants "div"
            |> Seq.filter
                (fun node -> 
                    match node.TryGetAttribute "id" with
                    |Some id -> id.Value().StartsWith("текст для проверки")
                    |None -> false)
            |> Seq.map
                (fun node -> node.InnerText())
        let next = getNextLink result
        if next.IsSome then
            yield! loop next.Value

    }
    loop start

let path = "http://адрес.html"

let values =
    getAllValues path
    |> Seq.concat

其余与接收数据的处理有关，与当前问题无关。对正确选择用于解析的库和实现本身感兴趣。

инспекция-кода

1 个回答

  1. Best Answer

    功能一般。但如果您追求完美，那么这里有一些提示：

    1. 所有处理都好像一起发生，一次坐着。所有代码都在一个带有子函数的大函数中，这使得代码的可读性和混淆性大大降低。我通常会尝试将代码分解成许多小函数，然后将它们连接在一起。这个想法是每个函数应该在一个层次上阅读，而不是几个层次。例如，“为每个 A 计算 B ”，而不是“为每个 A，其中 A 是执行 Z 的 Y 的所有 X，计算 B，其中 B 是 P 或 Q，取决于 I + J ”。人脑非常不擅长处理频繁的上下文切换。
    2. 类方法无处不在，这使得推断类型变得困难，迫使您显式指定类型，迫使您使用 lambda 表达式。我通常尝试将类方法包装成小函数，然后可以轻松对接。
    3. 参数化很少，所有行都硬连接到代码中。如果字符串"id","a"和"href"仍然可以理解为稳定标准的一部分，那么字符串"ключевое слово"和"текст для проверки"绝对应该是参数。
    4. 您的主循环产生一系列序列 - seq<seq<string>>，然后将其与Seq.concat. 这个操作对我来说似乎是多余的：因为你使用的是表达式seq { }，你可以立即使用yield!instead of 扩展其中的序列yield。
    5. 在这里我不确定这是否是您的要求，但是：如果页面上有多个带有文本“关键字”的链接会怎样？您的函数getNextLink只会返回第一个链接，而您将丢失其余部分。可以免费安排所有链接的处理，而不仅仅是第一个链接，只需Seq.tryHead从getNextLink. 但我不确定在你的情况下它是否会“更好”。

    这是我应用这些技巧后得到的结果：

    let attr name (node: HtmlNode) = node.TryGetAttribute name |> Option.map (fun v -> v.Value())
let text (node: HtmlNode) = node.InnerText()
let trim (s: string) = s.Trim()
let descendants tag (node: #HtmlNode) = node.Descendants tag
let startsWith prefix (s: string) = s.StartsWith prefix

let hasText value node = trim (text node) = value
let hasId value node = attr "id" |> Option.exists (startsWith value)

let getNextLink text = 
    descendants "a"
    >> Seq.filter (hasText text)
    >> Seq.choose (attr "href")
    >> Seq.tryHead

let linksOnPage idPrefix doc =
    descendants "div" 
    >> Seq.filter (hasId idPrefix)
    >> Seq.map text

let getAllValues linkText idPrefix start  = 
    let rec linksFromPageTree (url : string) = 
        seq {
            let doc = HtmlDocument.Load url
            yield! linksOnPage idPrefix doc
            yield! nextSubTree doc
        }
    and nextSubTree doc = 
        match getNextLink linkText doc with
        | None -> Seq.empty
        | Some nextUrl -> linksFromPageTree nextUrl

    linksFromPageTree start

let path = "http://адрес.html"
let values = getAllValues "ключевое слово" "текст для проверки" path
    • 6

相关问题