2015-10-03 12 views
9

Mam następującą funkcję, która sprawdza istnienie customer w źródło danych i zwraca identyfikator. Czy jest to właściwy/idiomatyczny sposób korzystania z typu Option?Czy to użycie opcji idiomatic w F #?

let findCustomerId fname lname email = 
    let (==) (a:string) (b:string) = a.ToLower() = b.ToLower() 
    let validFName name (cus:customer) = name == cus.firstname 
    let validLName name (cus:customer) = name == cus.lastname 
    let validEmail email (cus:customer) = email == cus.email 
    let allCustomers = Data.Customers() 
    let tryFind pred = allCustomers |> Seq.tryFind pred 
    tryFind (fun cus -> validFName fname cus && validEmail email cus && validLName lname cus) 
    |> function 
     | Some cus -> cus.id 
     | None -> tryFind (fun cus -> validFName fname cus && validEmail email cus) 
        |> function 
        | Some cus -> cus.id 
        | None -> tryFind (fun cus -> validEmail email cus) 
           |> function 
           | Some cus -> cus.id 
           | None -> createGuest() |> fun cus -> cus.id 

Odpowiedz

9

Nigdy nie jest dobrze, gdy wcinasz wcięcie, warto więc zobaczyć, co możesz z tym zrobić.

Oto jeden ze sposobów, aby rozwiązać ten problem, wprowadzając trochę funkcji pomocnika:

let tryFindNext pred = function 
    | Some x -> Some x 
    | None -> tryFind pred 

Można go używać wewnątrz funkcji findCustomerId spłaszczyć opcje zastępczych:

let findCustomerId' fname lname email = 
    let (==) (a:string) (b:string) = a.ToLower() = b.ToLower() 
    let validFName name (cus:customer) = name == cus.firstname 
    let validLName name (cus:customer) = name == cus.lastname 
    let validEmail email (cus:customer) = email == cus.email 
    let allCustomers = Data.Customers() 
    let tryFind pred = allCustomers |> Seq.tryFind pred 
    let tryFindNext pred = function 
     | Some x -> Some x 
     | None -> tryFind pred 
    tryFind (fun cus -> validFName fname cus && validEmail email cus && validLName lname cus) 
    |> tryFindNext (fun cus -> validFName fname cus && validEmail email cus) 
    |> tryFindNext (fun cus -> validEmail email cus) 
    |> function | Some cus -> cus.id | None -> createGuest().id 

to bardzo podobne do the approach outlined here.

+0

To podejście jest bardzo czytelne i łatwe do naśladowania, dziękuję! –

+2

Mniej ogólne, ale ogólnie lepsze podejście niż moje. +1 To powiedziawszy, '| Niektóre x -> Niektóre x' są potencjalnie marnotrawstwem - jeśli kompilator nie zoptymalizuje tego poprawnie, może to spowodować dodatkową alokację. '| Niektóre _ jako x -> x' byłyby tak jasne i potencjalnie bardziej wydajne. – ildjarn

+0

Zaid, dla graficznego obrazu zasad, które za tym stoją, spójrz na programowanie zorientowane na kolej, http://fsharpforfunandprofit.com/posts/recipe-part2/ –

5

Trochę abstrakcji może przejść długą drogę w zakresie czytelności ...

let bindNone binder opt = if Option.isSome opt then opt else binder() 

let findCustomerId fname lname email = 
    let allCustomers = Data.Customers() 
    let (==) (a:string) (b:string) = a.ToLower() = b.ToLower() 
    let validFName name (cus:customer) = name == cus.firstname 
    let validLName name (cus:customer) = name == cus.lastname 
    let validEmail email (cus:customer) = email == cus.email 
    let tryFind pred = allCustomers |> Seq.tryFind pred 
    tryFind (fun cus -> validFName fname cus && validEmail email cus && validLName lname cus) 
    |> bindNone (fun() -> tryFind (fun cus -> validFName fname cus && validEmail email cus)) 
    |> bindNone (fun() -> tryFind (fun cus -> validEmail email cus)) 
    |> bindNone (fun() -> Some (createGuest())) 
    |> Option.get 
    |> fun cus -> cus.id 

znacznie łatwiejsze do naśladowania, a jedyny koszt jest kilka dodatkowych null kontrole.

Również, gdybym był tobą, ponieważ większość z tych funkcji jest tak mała/trywialna, posypałbym ostrożnie inline.

3

Mówiąc o idiomatycznym użyciu języka, przede wszystkim F # promuje pisanie zwięzłego kodu, który wyraźnie odzwierciedla intencję. Patrząc na twój fragment z tego punktu widzenia, większość kodu jest nadmierna i ukrywa jedynie obserwację, że zwrócona wartość w żaden sposób nie zależy od firstname lub .

Twój fragment może być refactored do znacznie krótszy i znacznie jaśniejsze podobnej funkcji, które:

  • są podawane trzy argumenty ignoruje wszystkie, ale email,
  • potem kolejno wszystkich klientów próbuje znaleźć jeden mający (ignorując sprawę) tego samego email,
  • jeżeli stwierdzono, a następnie powraca do swojego id, othervise powraca createGuest().id

który niemal dosłownie przekłada się

let findCustomerId _ _ email = 
    Data.Customers() 
    |> Seq.tryFind (fun c -> System.String.Compare(email,c.email,true) = 0) 
    |> function Some(c) -> c.id | None -> createGuest().id 
9

opcje tworzą monady i są one również monoidal że wspierają one dwie funkcje postaci

zero: Option<T> 
combine: Option<T> -> Option<T> -> Option<T> 

wyrażenia obliczeniowe są używane do ładniejszy sposób pracy z monadami i wspierają również operacje monoid. Można zatem wdrożyć konstruktora obliczeń dla Option:

type OptionBuilder() = 
    member this.Return(x) = Some(x) 
    member this.ReturnFrom(o: Option<_>) = o 
    member this.Bind(o, f) = 
     match o with 
     | None -> None 
     | Some(x) -> f x 

    member this.Delay(f) = f() 
    member this.Yield(x) = Some(x) 
    member this.YieldFrom(o: Option<_>) = o 
    member this.Zero() = None 
    member this.Combine(x, y) = 
     match x with 
     | None -> y 
     | _ -> x 

let maybe = OptionBuilder() 

gdzie Combine zwraca pierwszy niepusty Option wartość.Następnie można użyć tego zaimplementować funkcję:

let existing = maybe { 
    yield! tryFind (fun cus -> validFName fname cus && validEmail email cus && validLName lname cus) 
    yield! tryFind (fun cus -> validFName fname cus && validEmail email cus) 
    yield! tryFind (fun cus -> validEmail email cus) 
} 
match existing with 
| Some(c) -> c.id 
| None -> (createGuest()).id 
+1

To jest naprawdę fajne rozwiązanie, ale trochę przesadzone dla mojej sytuacji . –

5

Po pierwsze, to nie mogą być bezpośrednio związane z pytaniem, ale może chcesz rearrage logikę w tej funkcji.

Zamiast:

"Patrzę dla klienta, który odpowiada FName, nazwisko i Emai; w przypadku jego braku, szukam tylko fname + e-mail, a potem po prostu e-mail, a następnie utworzyć gości"

może lepiej postępować tak:

„patrzę na email pasującym Jeżeli dostaję wiele odpowiedników, patrzę na pasującym fname, a jeśli jest wielokrotnością znowu patrzę na pasującym L-NAME” .

To nie tylko umożliwi lepszą strukturę kodu, ale zmusi cię do radzenia sobie z możliwymi problemami w logice.

Na przykład, jeśli masz wiele pasujących wiadomości e-mail, ale żadna z nich nie ma poprawnej nazwy? Obecnie wystarczy wybrać pierwszą sekwencję, która może być lub nie jest tym, czego potrzebujesz, w zależności od tego, w jaki sposób zamawiana jest Data.Customers(), , jeśli jest zamówiona,.

Teraz, jeśli wiadomości e-mail muszą być unikatowe, nie będzie to problemem - ale gdyby tak było, równie dobrze można pominąć sprawdzanie pierwszych/ostatnich nazw!

(Waham się o tym wspomnieć, ale może również nieco przyspieszyć twój kod, ponieważ nie musisz niepotrzebnie sprawdzać rekordów więcej niż jeden raz dla tych samych pól, ani nie sprawdzaj dodatkowych pól, gdy wystarczy tylko wiadomość e-mail.)

A teraz pytanie do ciebie - problem nie jest w użyciu Option, problem polega na tym, że wykonujesz zasadniczo tę samą operację trzy razy! ("Znajdź pasujące wyniki, a jeśli nie, znajdź rezerwę"). Refaktoryzacja funkcji w sposób rekurencyjny wyeliminuje brzydką strukturę przekątną, i pozwala na trywialne rozszerzenie funkcji w przyszłości, aby sprawdzić dodatkowe pola.

Niektóre inne drobne sugestie dotyczące kodzie:

  • Ponieważ tylko ty kiedykolwiek powołać się validFoo funkcji pomocniczych z tymi samymi argumentami dla Foo można upiec je w definicji funkcji odchudzić się w kodzie.
  • W przypadku rozróżniania wielkości znaków niewrażliwe na wielkość jest typowe, ale nieco nieoptymalne, ponieważ w rzeczywistości tworzy nowe małe kopie każdego ciągu. Właściwym sposobem jest użycie String.Equals(a, b, StringComparison.CurrentCultureIgnoreCase). W 99% jest to nieistotna mikrooptymalizacja, ale jeśli masz ogromną bazę klientów i wykonujesz wiele wyszukiwań dla klientów, jest to funkcja, która może mieć znaczenie!
  • Jeśli to możliwe, chciałbym zmodyfikować funkcję createGuest tak, że zwraca cały customer obiektu, a jedynie przyjąć .id jako ostatniej linii tej funkcji - albo jeszcze lepiej, zwracają customer z tej funkcji, jak również i oferować oddzielny jednolinijkowy findCustomerId = findCustomer >> (fun c -> c.id) dla łatwości użycia.

Z tym wszystkim mamy następujące.Dla przykładu, założę, że w przypadku wielu równorzędnych dopasowań, będziesz potrzebować ostatniego ostatniego lub najnowszego. Ale możesz również rzucić wyjątek, posortować według pola daty lub cokolwiek innego.

let findCustomerId fname lname email = 
    let (==) (a:string) (b:string) = String.Equals(a, b, StringComparison.CurrentCultureIgnoreCase) 
    let validFName = fun (cus:customer) -> fname == cus.firstname 
    let validLName = fun (cus:customer) -> lname == cus.lastname 
    let validEmail = fun (cus:customer) -> email == cus.email 
    let allCustomers = Data.Customers() 
    let pickBetweenEquallyValid = Seq.last 
    let rec check customers predicates fallback = 
     match predicates with 
     | [] -> fallback 
     | pred :: otherPreds -> 
      let matchingCustomers = customers |> Seq.filter pred 
      match Seq.length matchingCustomers with 
      | 0 -> fallback 
      | 1 -> (Seq.head matchingCustomers).id 
      | _ -> check matchingCustomers otherPreds (pickBetweenEquallyValid matchingCustomers).id    
    check allCustomers [validEmail; validFName; validLName] (createGuest()) 

Ostatnia sprawa: te brzydkie (i często O (n)) Seq.foo wyrażenia wszędzie są konieczne, ponieważ nie wiem jaki rodzaj sekwencji Data.Customers powraca, a ogólna Seq klasa nie jest bardzo przyjazny dla dopasowywanie do wzorca.

Jeśli, na przykład, Data.Customers zwraca tablicę, a następnie czytelność byłyby znacznie się poprawiła:

let pickBetweenEquallyValid results = results.[results.Length - 1] 
    let rec check customers predicates fallback = 
     match predicates with 
     | [] -> fallback 
     | pred :: otherPreds -> 
      let matchingCustomers = customers |> Array.filter pred 
      match matchingCustomers with 
      | [||] -> fallback 
      | [| uniqueMatch |] -> uniqueMatch.id 
      | _ -> check matchingCustomers otherPreds (pickBetweenEquallyValid matchingCustomers).id 
    check allCustomers [validEmail; validFName; validLName] (createGuest()) 
+0

Dzięki za wnikliwą i kompletną odpowiedź, logika, której użyłem, nie była najlepsza, ponieważ szybko zauważyłem, że złamałem zasadę DRY, ale ta funkcja służy wyłącznie do prototypowania; moje myśli były następujące: "napisz funkcję, która domaga się zwrotu * an *" id ", rozważę twoje eleganckie rozwiązanie dla wersji" produkcyjnej " –

2

Pozwól mi przeformułować i zmienić oświadczenie problem:

szukam 1) pasujące imię, nazwisko i adres e-mail, w którym to przypadku chciałbym zakończyć iterację. Po tym czasie tymczasowo przechowuję klienta z 2) pasującym imieniem i adresem e-mail lub, mniej korzystnie, 3) tylko pasującym adresem e-mail i dalej szukam 1). Elementy sekwencji powinny być ocenione najwyżej jeden raz.

Ten rodzaj problemu nie jest łatwy w obsłudze dla potokowych funkcji Seq, ponieważ obejmuje stan w rosnącej hierarchii, z zakończeniem po osiągnięciu najwyższego stanu. Zróbmy to więc w sposób imperatywny, zmieniając stan na zmienny, ale używając dyskryminowanej unii do kodowania i dopasowywania wzorców, aby uzyskać efekt przejścia stanu.

type MatchType<'a> = 
| AllFields of 'a 
| FNameEmail of 'a 
| Email of 'a 
| NoMatch 

let findCustomerId fname lname email = 
    let allCustomers = Data.Customers() 
    let (==) a b =   // Needs tweaking to pass the Turkey Test 
     System.String.Equals(a, b, System.StringComparison.CurrentCultureIgnoreCase) 
    let notAllFields = function AllFields _ -> false | _ -> true 
    let state = ref NoMatch 

    use en = allCustomers.GetEnumerator() 
    while notAllFields !state && en.MoveNext() do 
     let cus = en.Current 
     let fn = fname == cus.firstname 
     let ln = lname == cus.lastname 
     let em = email == cus.email 
     match !state with 
     | _     when fn && ln && em -> state := AllFields cus 
     | Email _ | NoMatch when fn && em  -> state := FNameEmail cus 
     | NoMatch   when em    -> state := Email cus 
     | _          ->() 

    match !state with 
    | AllFields cus 
    | FNameEmail cus 
    | Email cus -> cus.id 
    | NoMatch -> createGuest().id