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

F# inventory system

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

Problem

To learn F#, I've implemented this very simple inventory system. While I'm proud that that it's my first program, and that it works, there are still a few areas that I'd like tips on, namely these:

  • I don't like how I'm using a for ... in ... loop in Inventory.ChangeSelectedItem to find the length of Inventory.Items.



  • Is this the correct usage of F#'s type/class system? Should this be made in a more "functional" way?



  • Am I using getters/setters correctly? Do I need to declare the mutable variables internalName in my types, or is this automatically done?



  • Is my code properly styled?



```
open System

///
/// Represents an item. This type is only
/// used in the Inventory type.
///
/// The item's name.
/// The amount of this specific item.
type InventoryItem(name:string, count:int) =
let mutable internalCount = count

member this.Name = name
member this.Count
with get() = internalCount
and set(value) = internalCount
/// Increment or decrement how many items there are.
/// It will not allow for a negative count.
///
/// The amount to change the item count by.
member this.ChangeCount(amount) =
if this.Count >= 1 then
this.Count
/// This type represents an inventory, a collection
/// of values of the InventoryItem type.
///
/// A list of InventoryItems.
///
type Inventory(items:InventoryItem list, selectedItem:int) =
let mutable internalItems = items
let mutable internalSelectedItem = selectedItem

member this.Items
with get() = internalItems
and set(value:InventoryItem list) = internalItems
/// Add an item to the inventory.
///
/// The item to add.
member this.AddItem(item:InventoryItem) =
this.Items
/// Change the currently selected item.
///
/// The amount to increase by.
member this.ChangeSelectedItem(amount:int) =
let mutable itemCount = -1;
for _ in this.Items do
itemCount

Solution

Some things:

let mutable itemCount = -1;
    for _ in this.Items do
        itemCount <- itemCount + 1


can become

let itemCount = this.Items |> List.length


In terms of types, in F# I would probably make Inventory Item a record rather than a class.

The easiest way to know if something needs to be mutable and you are stuck is to leave it off and see if the compiler complains.

member this.ChangeCount(amount) =
    if this.Count >= 1 then
        this.Count <- this.Count + amount


should probably be

member this.ChangeCount(amount) =
    if this.Count+amount >= 1 then
        this.Count <- this.Count + amount

Code Snippets

let mutable itemCount = -1;
    for _ in this.Items do
        itemCount <- itemCount + 1
let itemCount = this.Items |> List.length
member this.ChangeCount(amount) =
    if this.Count >= 1 then
        this.Count <- this.Count + amount
member this.ChangeCount(amount) =
    if this.Count+amount >= 1 then
        this.Count <- this.Count + amount

Context

StackExchange Code Review Q#97642, answer score: 7

Revisions (0)

No revisions yet.