Reputation: 869
I'm trying to learn F# and I feel like I can write / rewrite this block of code to be more "idiomatic" F# but I just can't figure out how I can accomplish it.
My simple program will be loading values from 2 csv files: A list of Skyrim potion effects, and a list of Skyrim Ingredients. An ingredient has 4 Effects. Once I have the Ingredients, I can write something to process them - right now, I just want to write the CSV load in a way that makes sense.
Code
Here are my types:
type Effect(name:string, id, description, base_cost, base_mag, base_dur, gold_value) =
member this.Name = name
member this.Id = id
member this.Description = description
member this.Base_Cost = base_cost
member this.Base_Mag = base_mag
member this.Base_Dur = base_dur
member this.GoldValue = gold_value
type Ingredient(name:string, id, primary, secondary, tertiary, quaternary, weight, value) =
member this.Name = name
member this.Id = id
member this.Primary = primary
member this.Secondary = secondary
member this.Tertiary = tertiary
member this.Quaternary = quaternary
member this.Weight = weight
member this.Value = value
Here is where I parse an individual comma-separated string, per type:
let convertEffectDataRow (csvLine:string) =
let cells = List.ofSeq(csvLine.Split(','))
match cells with
| name::id::effect::cost::mag::dur::value::_ ->
let effect = new Effect(name, id, effect, Decimal.Parse(cost), Int32.Parse(mag), Int32.Parse(dur), Int32.Parse(value))
Success effect
| _ -> Failure "Incorrect data format!"
let convertIngredientDataRow (csvLine:string) =
let cells = List.ofSeq(csvLine.Split(','))
match cells with
| name::id::primary::secondary::tertiary::quaternary::weight::value::_ ->
Success (new Ingredient(name, id, primary, secondary, tertiary, quaternary, Decimal.Parse(weight), Int32.Parse(value)))
| _ -> Failure "Incorrect data format!"
So I feel like I should be able to build a function that accepts one of these functions or chains them or something, so that I can recursively go through the lines in the CSV file and pass those lines to the correct function above. Here is what I've tried so far:
type csvTypeEnum = effect=1 | ingredient=2
let rec ProcessStuff lines (csvType:csvTypeEnum) =
match csvType, lines with
| csvTypeEnum.effect, [] -> []
| csvTypeEnum.effect, currentLine::remaining ->
let parsedLine = convertEffectDataRow2 currentLine
let parsedRest = ProcessStuff remaining csvType
parsedLine :: parsedRest
| csvTypeEnum.ingredient, [] -> []
| csvTypeEnum.ingredient, currentLine::remaining ->
let parsedLine = convertIngredientDataRow2 currentLine
let parsedRest = ProcessStuff remaining csvType
parsedLine :: parsedRest
| _, _ -> Failure "Error in pattern matching"
But this (predictably) has a compile error on second instance of recursion and the last pattern. Specifically, the second time parsedLine :: parsedRest
shows up does not compile. This is because the function is attempting to both return an Effect
and an Ingredient
, which obviously won't do.
Now, I could just write 2 entirely different functions to handle the different CSVs, but that feels like extra duplication. This might be a harder problem than I'm giving it credit for, but it feels like this should be rather straightforward.
Sources
The CSV parsing code I took from chapter 4 of this book: https://www.manning.com/books/real-world-functional-programming
Upvotes: 1
Views: 110
Reputation: 12184
Since the line types aren't interleaved into the same file and they refer to different csv file formats, I would probably not go for a Discriminated Union and instead pass the processing function to the function that processes the file line by line.
In terms of doing things idiomatically, I would use a Record rather than a standard .NET class for this kind of simple data container. Records provide automatic equality and comparison implementations which are useful in F#.
You can define them like this:
type Effect = {
Name : string; Id: string; Description : string; BaseCost : decimal;
BaseMag : int; BaseDuration : int; GoldValue : int
}
type Ingredient= {
Name : string; Id: string; Primary: string; Secondary : string; Tertiary : string;
Quaternary : string; Weight : decimal; GoldValue : int
}
That requires a change to the conversion function, e.g.
let convertEffectDataRow (csvLine:string) =
let cells = List.ofSeq(csvLine.Split(','))
match cells with
| name::id::effect::cost::mag::dur::value::_ ->
Success {Name = name; Id = id; Description = effect; BaseCost = Decimal.Parse(cost);
BaseMag = Int32.Parse(mag); BaseDuration = Int32.Parse(dur); GoldValue = Int32.Parse(value)}
| _ -> Failure "Incorrect data format!"
Hopefully it's obvious how to do the other one.
Finally, cast aside the enum
and simply replace it with the appropriate line function (I've also swapped the order of the arguments).
let rec processStuff f lines =
match lines with
|[] -> []
|current::remaining -> f current :: processStuff f remaining
The argument f
is just a function that is applied to each string line. Suitable f
values are the functions we created above, e.g.convertEffectDataRow
. So you can simply call processStuff convertEffectDataRow
to process an effect file and processStuff convertIngredientDataRow
to process and ingredients file.
However, now we've simplified the processStuff
function, we can see it has type: f:('a -> 'b) -> lines:'a list -> 'b list
. This is the same as the built-in List.map
function so we can actually remove this custom function entirely and just use List.map
.
let processEffectLines lines = List.map convertEffectDataRow lines
let processIngredientLines lines = List.map convertIngredientDataRow lines
Upvotes: 2
Reputation: 35144
ProcessStuff
returns a list from one case, but a single item (Failure
) from the other case. Thus compilation error.You haven't shown what Success
and Failure
definitions are. Instead of generic success, you could define the result as
type Result =
| Effect of Effect
| Ingredient of Ingredient
| Failure of string
And then the following code compiles correctly:
let convertEffectDataRow (csvLine:string) =
let cells = List.ofSeq(csvLine.Split(','))
match cells with
| name::id::effect::cost::mag::dur::value::_ ->
let effect = new Effect(name, id, effect, Decimal.Parse(cost), Int32.Parse(mag), Int32.Parse(dur), Int32.Parse(value))
Effect effect
| _ -> Failure "Incorrect data format!"
let convertIngredientDataRow (csvLine:string) =
let cells = List.ofSeq(csvLine.Split(','))
match cells with
| name::id::primary::secondary::tertiary::quaternary::weight::value::_ ->
Ingredient (new Ingredient(name, id, primary, secondary, tertiary, quaternary, Decimal.Parse(weight), Int32.Parse(value)))
| _ -> Failure "Incorrect data format!"
type csvTypeEnum = effect=1 | ingredient=2
let rec ProcessStuff lines (csvType:csvTypeEnum) =
match csvType, lines with
| csvTypeEnum.effect, [] -> []
| csvTypeEnum.effect, currentLine::remaining ->
let parsedLine = convertEffectDataRow currentLine
let parsedRest = ProcessStuff remaining csvType
parsedLine :: parsedRest
| csvTypeEnum.ingredient, [] -> []
| csvTypeEnum.ingredient, currentLine::remaining ->
let parsedLine = convertIngredientDataRow currentLine
let parsedRest = ProcessStuff remaining csvType
parsedLine :: parsedRest
| _, _ -> [Failure "Error in pattern matching"]
csvTypeEnum
type looks fishy, but I'm not sure what you were trying to achieve, so just fixed the compilation errors.
Now you can refactor your code to reduce duplication by passing functions as parameters when needed. But always start with types!
Upvotes: 2
Reputation: 6324
You can certainly pass a function to another function and use a DU as a return type, for example:
type CsvWrapper =
| CsvA of string
| CsvB of int
let csvAfunc x =
CsvA x
let csvBfunc x =
CsvB x
let csvTopFun x =
x
csvTopFun csvBfunc 5
csvTopFun csvAfunc "x"
As for the type definitions, you can just use records, will save you some typing:
type Effect = {
name:string
id: int
description: string
}
let eff = {name="X";id=9;description="blah"}
Upvotes: 0