patternMinor
CRUD (database layer) for F# with TypeProviders
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:
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
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.Record2Itemtry
db.DataContext.SubmitChanges()
with
| exn -> printfn "Exception: \n%s" exn.MessageAre 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.Record2Itemtry
db.DataContext.SubmitChanges()
with
| exn -> printfn "Exception: \n%s" exn.MessageContext
StackExchange Code Review Q#82260, answer score: 6
Revisions (0)
No revisions yet.