HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

CRUD (database layer) for F# with TypeProviders

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
layerwithdatabasetypeprovidersforcrud

Problem

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

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

Solution

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.

Code Snippets

new() = new Item()
query {
    for row in db.ItemsTable do
    select row
}
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)
|> Seq.tryPick Some
|> Option.map this.Record2Item
try
    db.DataContext.SubmitChanges()
with
    | exn -> printfn "Exception: \n%s" exn.Message

Context

StackExchange Code Review Q#82260, answer score: 6

Revisions (0)

No revisions yet.