CRUD (database layer) for F# with TypeProviders

Question

I am a newbie in F# and I am thinking about use F# in my next project. The project will work the database and I need to store and retrieve instances of objects in the Db.

Could you please check my code and say if there is something that I can improve?

Here is a simple object:

namespace Entities

type Item(id : string, name : string) = 
    member val ItemId = id with get, set
    member val ItemName = name with get, set

    new() = new Item()

Here is Database Layer:

namespace SqlController

open System
open System.Data
open System.Data.Linq
open Microsoft.FSharp.Data.TypeProviders
open Microsoft.FSharp.Linq

open Entities

type dbSchema = SqlDataConnection<"Data Source=VM-WIN8\SQLEXPRESS;Initial Catalog=mobile;Integrated Security=SSPI;">

type SqlConnector() = 

    // universal sql functions
    member private this.DeleteRowsFrom (table : Table<_>) rows =
        table.DeleteAllOnSubmit(rows)

    // map functions
    member private this.Item2Record (item : Item) = 
        new dbSchema.ServiceTypes.ItemsTable(
            ItemId = item.ItemId, 
            ItemName = item.ItemName)

    member private this.Record2Item (e : dbSchema.ServiceTypes.ItemsTable) =         
        new Item(e.ItemId, e.ItemName)

    // ### Items

    // return all items 
    member this.GetItems() = 
        use db = dbSchema.GetDataContext()             

        query {
            for row in db.ItemsTable do
            select row
        } |> Seq.map this.Record2Item |> Seq.toArray


    // return one item by item id
    member this.GetItemByItemId(itemId : string) = 
        use db = dbSchema.GetDataContext()

        query {
            for rows in db.ItemsTable do
            where (rows.ItemId = itemId)
            select rows
        } 
        |> (fun s ->
                if Seq.isEmpty s then
                    None
                else
                    s |> Seq.head |> this.Record2Item |> Some)

    // insert new item
    member this.InsertItem (item : Item) =                     
        use db = dbSchema.GetDataContext() 
        item |> this.Item2Record |> db.ItemsTable.InsertOnSubmit

        try
            db.DataContext.SubmitChanges()
        with
            | exn -> printfn "Exception: \n%s" exn.Message

    // update item
    member this.UpdateItem (item : Item) = 
        use db = dbSchema.GetDataContext() 

        query {
            for rows in db.ItemsTable do
            where (rows.ItemId = item.ItemId)
            select rows
        } 
        |> Seq.nth 0 
        |> (fun e -> e.ItemName <- item.ItemName)

        try
            db.DataContext.SubmitChanges()
        with
            | exn -> printfn "Exception: \n%s" exn.Message

    // delete item by itemId
    member this.DeleteItemByItemId(itemId : string) =
        use db = dbSchema.GetDataContext() 

        query {
            for rows in db.ItemsTable do
            where (rows.ItemId = itemId)
            select rows
        } |> this.DeleteRowsFrom db.ItemsTable

        try
            db.DataContext.SubmitChanges()
        with
            | exn -> printfn "Exception: \n%s" exn.Message

Here is some tests:

module Tests

open NUnit.Framework
open FsUnit

open Entities
open SqlController

[<TestFixture>]
type SqlControllerTest() =
    let sqlConnector = new SqlConnector();  

    [<Test>]
    member x.SelectionAllTest() = 
        let items = sqlConnector.GetItems()
        items.Length |> should equal 3

    [<Test>]
    member x.SelectById() =
        let item = sqlConnector.GetItemByItemId("20")
        if Option.isSome item then
            (Option.get item).ItemName |> should equal "two"

    [<Test>]
    member x.SelectNotExistingItem() =
        let item = sqlConnector.GetItemByItemId("xx")
        Option.isNone item |> should equal true

Answer 1

new() = new Item()

This is infinite recursion, in order to call the parameterless constructor, you call the parameterless constructor. It's not quite clear to me what the default values of id and name should be. In C#, it might be okay to use null, but that's not the common approach in F#.

---

query {
    for row in db.ItemsTable do
    select row
}

Wouldn't just db.ItemsTable work here too?

---

query {
    for rows in db.ItemsTable do
    where (rows.ItemId = itemId)
    select rows
} 
|> (fun s ->
        if Seq.isEmpty s then
            None
        else
            s |> Seq.head |> this.Record2Item |> Some)

That second part could be simplified to:

|> Seq.tryPick Some
|> Option.map this.Record2Item

---

try
    db.DataContext.SubmitChanges()
with
    | exn -> printfn "Exception: \n%s" exn.Message

Are you sure that it's okay for your DB layer to just print any errors to the console and then continue as if everything was fine? That doesn't sound like a very robust approach to me.

If you're sure you want to do it this way, then consider extracting this code into a separate function, since you're repeating it at several places.

---

|> (fun e -> e.ItemName <- item.ItemName)

I personally don't like this style of code. Pipelining using |> should be used for code that doesn't mutate anything.

---

if Option.isSome item then
    (Option.get item).ItemName |> should equal "two"

I would probably use pattern matching here.