Query a product from a list of brands

Question

I am quite new to F# and just wrote my first program. It checks if the query exists in the brand list and returns the matching brand.

Query is the string you are searching for in the brand list. So someone could be looking for "Miso Power Washer X1000" the idea is that my function analyses the query to see if there is a matching brand. Later the remainder ("Power Washer X1000") could be parsed as well. In this way I am able to "parse" the query of the user and give it context. This meta data will be used to give better search results when the database is queried.

I used stringInArray in order to split the logic into a separate function. Of course I could do something like Array.filter(fun (elem: string) -> query.IndexOf(elem) > -1)) brands But I felt it was less readable.

Sometimes when learning a new language you get things to work. However someone more experienced say: you can use this or that which is more common, faster etc. I am looking for this kind of feedback before creating a bunch of code which turns out to be rubbish through the eyes of an experienced F# programmer.

Is this the correct way to use F# function structure?

Code

module Program =

    [<EntryPoint>]
    let main argv =
        let query = "Miso Power Washer X1000"

        let brands = [|
            "Hayo"
            "Miso"
            "The Master"
            "Vector"
        |]

        let stringInArray = fun (elem: string) -> 
            query.IndexOf(elem) > -1

        let getBrands (query: string): string[] =
            Array.filter(stringInArray) brands

        let result = getBrands query
        printfn "%A" result

        0

Output

[|"Miso"|]

Answer 1

Unfortunately, there is at least one blatant issue with the program in its current state: unused arguments. Let's have a look at it and find out why that's important.

Before that, a short disclaimer, though: I'm not a F# developer. However, I know functional programming (e.g. Haskell). I don't know the .NET lands by heart. Take this review with a grain of salt on the arguments that concern F#.

Functions and the world

When we write a function in a context, it gains knowledge of that context. For example, we bound five to addFive in the following example:

let y = 5
let addFive x = x + y

However, there is a possible issue with the code above: we might accidentally use y at a place we didn't intend, for example:

let add x z = x + y

This is exactly what happened in your functions:

        let stringInArray = fun (elem: string) -> 
            query.IndexOf(elem) > -1

        let getBrands (query: string): string[] =
            Array.filter(stringInArray) brands

Note how stringInArray just uses query? And how getBrands completely ignores the given query? This means that we could use let result = getBrands "" and still end up with [|"Miso"|]. That's not what we intended!

Instead, let's go back to back to the drawing board. We need to make sure that the query gets used. So we need to add at least one argument to stringInArray:

        let stringInArray (query: string) (elem: string) = 
            query.IndexOf(elem) > -1

Now we can use query in getBrands:

        let getBrands query =
            Array.filter(stringInArray query) brands

Great! Now let result = getBrands "" leads to an empty array. Success!

Names and tales

However, now that we changed stringInArray, we note that the name isn't quite fitting: if we add type signatures, we and up with:

        let stringInArray (query: string) (elem: string) = 
            query.IndexOf(elem) > -1

Neither of the arguments is an array. We should call this function contains or similar. However, we could introduce another function that gets matching elements from an array:

        let isSubstringOf (haystack: string) (needle: string) =
            haystack.IndexOf(needle) > -1

        let matchingElements arr haystack =
            Array.filter(isSubstringOf haystack) arr

        let getBrands query = matchingElements brands query
        // or even
        //  getBrands = matchingElements brands

Note that with this approach we can keep the definition of getBrands to a minimum:

        let getBrands = matchingElements brands

General purpose functions and the world

Now that we used proper naming and split the functionality of our functions, it's time to re-evaluate whether they really belong in main. Remember how functions have their context saved? They provide a closure. It's therefore a good idea to keep the context small.

What functions should we therefore move out of main? We have the following at hand:

• isSubstringOf, which searches in a string for another string
• matchingElements, which filters an array of strings whether they are contained in the second argument
• getBrands, which filters brands given a query

The first two functions sound very generic, so let's move them out of main:

module Program =

    let isSubstringOf (haystack: string) (needle: string) =
        haystack.IndexOf(needle) > -1

    let matchingElements arr haystack =
        Array.filter(isSubstringOf haystack) arr

    [<EntryPoint>]
    let main argv =
        let query = "Miso Power Washer X1000"

        let brands = [|
            "Hayo"
            "Miso"
            "The Master"
            "Vector"
        |]

        let getBrands = matchingElements brands

        let result = getBrands query
        printfn "%A" result

        0

Note how short our main got. It only contains the essential elements: query, brands, getBrands and result. One could argue that we can just replace getBrands by its definition, but premature brevity in source code is the source of future confusion, so let's keep it a little bit more verbose but self-explanatory.

Moving the functions might seem like an overkill, but note that this approach immediately had shown an error if we followed it right from the beginning. If we now use we now use query accidentally in isSubstringOf, we immediately get a compiler error (and probably an IntelliJ warning/note/error). That can be a huge boon in finding errors!

Furthermore, this approach makes it easy to unit test the functions later. Maybe we want to improve the isSubstringOf to use fuzzy logic so that it also works for "Mizo Power Washer". (Unit) tests can make sure that we don't accidentally break old functionality on the way.

Answer 2

Very brittle solution (for example, case mismatch, e.g. "miso" vs. "Miso", will fail to retrieve the required answer), but short and to the point (with plenty of room to improve robustness):

let brands = ["Hayo"; "Miso"; "The Master"; "Vector"]
let product = "Miso power vacuum X100"
List.collect (fun (elem: string) ->
                    if product.Contains(elem) then
                        [elem]
                    else
                        []) brands |> printfn "%A"

P.S. Some may say otherwise, but I feel lists are more idiomatic to F# than arrays.