Как уменьшить дублирование кода с помощью вложенных операторов if? - PullRequest
Новая
       5

Как уменьшить дублирование кода с помощью вложенных операторов if?

6 голосов
/ 09 марта 2020

давайте рассмотрим этот код: 

let getBuildDate (assembly: Assembly) : DateTime option =

    let buildVersionMetadataPrefix = "+build"
    let attribute = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()

    if attribute <> null && attribute.InformationalVersion <> null then
        let value = attribute.InformationalVersion
        let index = value.IndexOf(buildVersionMetadataPrefix)
        if index > 0 then
            let value = value.Substring(index + buildVersionMetadataPrefix.Length)
            let success, timestamp = DateTime.TryParseExact(value, "yyyyMMddHHmmss", CultureInfo.InvariantCulture, DateTimeStyles.None)
            if success then
                Some timestamp
            else
                None
        else
            None
    else
        None

Есть ли способ избавиться от всех операторов 'else None', чтобы иметь только одно?

С одной стороны, я могу представить что для некоторых людей код более понятен благодаря изложению всех утверждений None, но с другой стороны, происходящих из мира C, я вижу его как беспорядок, снижающий читабельность.

Есть много случаев где вам нужно выполнить ряд условий и все невыполненные дела go в одном месте.

Если у меня есть список условий, которые зависят от успеха друг друга, как я могу сделать краткую краткую выход без дублирования.

Ответы [ 5 ]

8 голосов
/ 09 марта 2020

Другим подходом может быть использование функций Option - каждый из этих шагов будет эффективно замыкаться, если на предыдущем шаге введено значение None. 

let getBuildDate (assembly: Assembly) : DateTime option =    
    let tryDate value =
         match DateTime.TryParseExact(value, "yyyyMMddHHmmss", CultureInfo.InvariantCulture, DateTimeStyles.None) with
         | true, date -> Some date
         | false, _ -> None

    let buildVersionMetadataPrefix = "+build"
    let attribute = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()

    Option.ofObj attribute
    |> Option.bind (fun attr -> Option.ofObj attr.InformationalVersion)
    |> Option.map (fun infVer -> infVer, infVer.IndexOf buildVersionMetadataPrefix)
    |> Option.filter (fun (_, index) -> index > 0)
    |> Option.map (fun (infVer, index) -> infVer.Substring(index + buildVersionMetadataPrefix.Length))
    |> Option.bind tryDate

Является ли это «лучше» спорно - и, безусловно, зависит от точки зрения

7 голосов
/ 09 марта 2020

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

Однако, если вам нужен простой способ изменить код, чтобы поток управления был более понятным ( не делая это более умным), я бы отрицал условия. Ранее у вас было: 

if something then 
  moreStuff()
  Some result
else
  None

Вы можете переписать это, вернув None, если not something. Я думаю, что соглашение о кодировании F # в этом случае также позволяет вам удалять отступы, так что это больше похоже на обязательный досрочный возврат: 

if not something then None else
moreStuff()
Some result

При этом вы можете написать свою исходную функцию следующим образом - без каких-либо дополнительных умные трюки: 

let getBuildDate (assembly: Assembly) : DateTime option =

    let buildVersionMetadataPrefix = "+build"
    let attribute = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()

    if attribute = null || attribute.InformationalVersion = null then None else
    let value = attribute.InformationalVersion
    let index = value.IndexOf(buildVersionMetadataPrefix)
    if index <= 0 then None else
    let value = value.Substring(index + buildVersionMetadataPrefix.Length)
    let success, timestamp = DateTime.TryParseExact(value, "yyyyMMddHHmmss", CultureInfo.InvariantCulture, DateTimeStyles.None)
    if not success then None else
    Some timestamp
3 голосов
/ 09 марта 2020

Удобный для чтения подход может использовать построитель выражений вычислений для Option. 

type OptionBuilder() =
    member _.Return v = Some v
    member _.Zero () = None
    member _.Bind(v, f) = Option.bind f v
    member _.ReturnFrom o = o

let opt = OptionBuilder()

. Вы можете смоделировать императивный стиль if-then-return. 

let condition num = num % 2 = 0

let result = opt {
    if condition 2 then 
        if condition 4 then 
            if condition 6 then 
                return 10
}

Переписав свой пример : 

let getBuildDate (assembly: Assembly) : DateTime option = opt {

    let buildVersionMetadataPrefix = "+build"
    let attribute = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()

    if attribute <> null && attribute.InformationalVersion <> null then
        let value = attribute.InformationalVersion
        let index = value.IndexOf(buildVersionMetadataPrefix)
        if index > 0 then
            let value = value.Substring(index + buildVersionMetadataPrefix.Length)
            let success, timestamp = DateTime.TryParseExact(value, "yyyyMMddHHmmss", CultureInfo.InvariantCulture, DateTimeStyles.None)
            if success then
                return timestamp
}

Не более None.

1 голос
/ 11 марта 2020

Рассматривали ли вы использование Result<TSuccess, TError>. Это очень структурирует - делает код жестким и плоским - и позволяет предоставить подробную информацию об ошибках для шага, который возможен неудачно. Это немного больше кода, но IMO более читабелен и удобен в обслуживании: 

let getBuildDate (assembly: Assembly) : Result<DateTime, string> = 

    let buildVersionMetadataPrefix = "+build"

    let extractAttribute (assem: Assembly) = 
        match assem.GetCustomAttribute<AssemblyInformationalVersionAttribute>() with
        | attrib when attrib <> null -> Ok attrib
        | _ -> Error "No attribute found"

    let extractDateString (attrib: AssemblyInformationalVersionAttribute) =
        match attrib.InformationalVersion.IndexOf (buildVersionMetadataPrefix) with
        | x when x > 0 -> Ok (attrib.InformationalVersion.Substring (x + buildVersionMetadataPrefix.Length))
        | _ -> Error "Metadata prefix not found"

    let toDateTime dateString =
        match DateTime.TryParseExact(dateString, "yyyyMMddHHmmss", CultureInfo.InvariantCulture, DateTimeStyles.None) with
        | true, timeStamp -> Ok timeStamp
        | false, _ -> Error "Invalid date time format"

    extractAttribute assembly
    |> Result.bind extractDateString
    |> Result.bind toDateTime

Использование 

let optBuildDate = getBuildDate (Assembly.GetExecutingAssembly())
match optBuildDate with
| Ok date -> printfn "%A" date
| Error msg -> printfn "ERROR: %s" msg
1 голос
/ 09 марта 2020 
open System
open System.Reflection
open System.Globalization

let inline guard cond next = if cond then next () else None

let getBuildDate (assembly: Assembly) : DateTime option =
    let buildVersionMetadataPrefix = "+build"
    let attribute = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()

    guard (attribute <> null && attribute.InformationalVersion <> null) <| fun _ ->
    let value = attribute.InformationalVersion
    let index = value.IndexOf(buildVersionMetadataPrefix)
    guard (index > 0) <| fun _ ->
    let value = value.Substring(index + buildVersionMetadataPrefix.Length)
    let success, timestamp = DateTime.TryParseExact(value, "yyyyMMddHHmmss", CultureInfo.InvariantCulture, DateTimeStyles.None)
    guard success <| fun _ ->
    Some timestamp

Если вы можете пережить неуместность необходимости писать <| fun _ -> на каждом guard, этот вариант стоит рассмотреть.

...