Рефакторинг маленькой функции F # - PullRequest
Новая
       9

Рефакторинг маленькой функции F #

3 голосов
/ 21 августа 2011

Я сделал следующую функцию F #, которая будет получать URL-адрес из HTML-содержимого веб-страницы: 

let getPicUrl (urlContents : string) =
  let START_TOKEN = "jpg_url="
  let startIndex = urlContents.IndexOf(START_TOKEN)
  let endIndex = urlContents.IndexOf("&amp", startIndex)
  let s = startIndex + START_TOKEN.Length
  let l = endIndex-startIndex-START_TOKEN.Length

  urlContents.Substring(s, l)

На самом деле последняя строка, urlContents.Substring(s, l), нужна только s и l, поэтому мне было интересно, смогу ли я преобразовать части этой функции в некоторые внутренние функции, чтобы мои намерения были более ясными. В идеале getPicUrl будет иметь только 2 let инструкции, s и l, а все остальные будут внутренними определениями для этих let инструкций. Если это может быть каким-либо образом достигнуто или нет, это другая история ..

Единственный очевидный способ улучшения кода, который я могу представить на данный момент, - это поменять местами endIndex, чтобы мы получили 

let getPicUrl (urlContents : string) =
  let START_TOKEN = "jpg_url="
  let startIndex = urlContents.IndexOf(START_TOKEN)
  let s = startIndex + START_TOKEN.Length
  let l =
    let endIndex = urlContents.IndexOf("&amp", startIndex)
    endIndex-startIndex-START_TOKEN.Length

  urlContents.Substring(s, l)

но я продолжаю задаваться вопросом, был бы более ясный способ организовать определения let этой функции.

Ответы [ 3 ]

3 голосов
/ 21 августа 2011

Во-первых, ваша функция глючит.Несоответствующая строка сделает ее раздражительной.

Мне нравятся регулярные выражения для такого рода вещей.С этим активным шаблоном: 

open System.Text.RegularExpressions

let (|Regex|_|) pattern input =
  let m = Regex.Match(input, pattern)
  if m.Success then Some(List.tail [for g in m.Groups -> g.Value])
  else None

вы можете сделать: 

let tryGetPicUrl = function
  | Regex @"jpg_url=([^&]+)&amp" [url] -> Some url
  | _ -> None

Вы также можете превратить свой исходный подход в активный шаблон:: 

let tryGetPicUrl = function
  | Between "jpg_url" "&amp" url -> Some url
  | _ -> None
2 голосов
/ 21 августа 2011

Вы можете написать это так: 

let getPicUrl (urlContents : string) =
  let s =
    let START_TOKEN = "jpg_url="
    let startIndex = urlContents.IndexOf(START_TOKEN)
    startIndex + START_TOKEN.Length
  let l =
    let endIndex = urlContents.IndexOf("&amp", s)
    endIndex-s

  urlContents.Substring(s, l)
0 голосов
/ 21 августа 2011

Другим вариантом будет использование метода разбиения строки (я надеюсь, что строка не слишком длинная, поскольку это может повлиять на производительность) и использование типа параметра, чтобы указать, был ли найден URL-адрес. 

let getPicUrl (urlContents : string) =
    let splitAndGet n (sep:string) (str:string) = 
        let spl = str.Split([|sep|],StringSplitOptions.None)
        match spl.Length with
        | x when x > n -> Some (spl.[n])
        | _ -> None 
    match urlContents |> splitAndGet 1 "jpg_url=" with
    | Some str -> str |> splitAndGet 0 "&amp"
    | _ -> None
...