F# inventory system - Part 2

Question

I've refactored my previous inventory system, and added a few features like removing items from the Inventory class, easily obtaining the current selected item through Inventory.GetSelectedItem, and some things have been renamed as well. My concerns are once again:

• 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?

Item.fs

namespace InventorySystem
    open System

    /// <summary>
    /// Represents an item. This type is only
    /// with the Inventory type.
    /// </summary>
    /// <param name="name">The item's name.</param>
    /// <param name="count">The amount of this specific item.</param>
    type Item(name: string, count: int) =
        let mutable _count = count
        let mutable _name = name

        member this.Name
            with get() = _name
            and set(value: string) = _name <- value

        member this.Count
            with get() = _count
            and set(value: int) = _count <- value

        /// <summary>
        /// Increment or decrement the how much of this
        /// specific item you have.
        /// </summary>
        /// <param name="amount">The amount to change by.</param>
        member this.ChangeCount(amount: int) =
            if this.Count + amount >= 0 then
                this.Count <- this.Count + amount
            else
                printfn "You cannot have less than zero items."

        /// <summary>
        /// Return this type in the following format:
        /// Name={item name}, Count={item count}.
        /// </summary>
        override this.ToString() =
            String.Format("Name={0}, Count={1}", this.Name, this.Count)

Inventory.fs

namespace InventorySystem
    open System

    /// <summary>
    /// This type represents an inventory, containing items.
    /// </summary>
    /// <param name="items">A list of items.</param>
    type Inventory(items: Item list) =
        let mutable _items = items
        let mutable _selectedItem = 0

        member this.Items
            with get() = _items
            and set(value: Item list) = _items <- value

        member this.SelectedItem
            with get() = _selectedItem
            and set(value: int) = _selectedItem <- value

        /// <summary>
        /// Add an item to the list of items.
        /// </summary>
        /// <param name="item">The item to add.</param>
        member this.AddItem(item: Item) =
            this.Items <- item :: this.Items

        /// <summary>
        /// Remove an item from the inventory based
        /// the item's index.
        /// </summary>
        /// <param name="itemIndex">The index of the item to remove.</param>
        member this.RemoveItem(itemIndex: int) =
            let mutable resultList = []

            for index = 0 to this.Items.Length - 1 do
                if index <> itemIndex then
                    resultList <- this.Items.[index] :: resultList

            this.Items <- resultList

        /// <summary>
        /// Change the currently selected item.
        /// </summary>
        /// <param name="amount">The amount to change the selected item by.</param>
        member this.ChangeSelectedItem(amount: int) =
            if this.SelectedItem + amount < this.Items.Length && this.SelectedItem + amount >= 0 then
                this.SelectedItem <- this.SelectedItem + amount
            else
                printfn "The currently selected item must stay in the range 0 -> Items.Length."

        /// <summary>
        /// Get the currently selected item.
        /// </summary>
        member this.GetSelectedItem() =
            this.Items.[this.SelectedItem]

        /// <summary>
        /// Return this type in the following format:
        /// Items={items}, Selected item={selected item}
        /// </summary>
        override this.ToString() =
            String.Format("Items={0}, Selected Item={1}", this.Items, this.Items.[this.SelectedItem])

Test.fs

open System
open InventorySystem

[<EntryPoint>]
let main argv =
    let myInventory = new Inventory([
        new Item("Sword of Death", 1);
        new Item("Gold", 1)
    ])

    myInventory.AddItem(new Item("Silver", 5))
    myInventory.AddItem(new Item("Copper", 10))
    Console.WriteLine(myInventory)

    myInventory.RemoveItem(1)
    Console.WriteLine(myInventory)

    Console.WriteLine(myInventory.GetSelectedItem())
    myInventory.ChangeSelectedItem(1)
    Console.WriteLine(myInventory.GetSelectedItem())

    Console.WriteLine(myInventory)
    Console.ReadKey()
    0

Answer 1

To answer your first question "Should this be made in a more "functional" way?"

1. Embrace immutability.
2. Prefer algebraic types and records over classes.

Here is my version:

type InventoryItem = {name:string}
type Inventory = Inventory of (InventoryItem * int) list

[<CompilationRepresentation (CompilationRepresentationFlags.ModuleSuffix)>]
module Inventory =

   let create items =
      Inventory items

   let countItem item (Inventory inventory) =
     inventory 
     |> List.tryFind (fun t -> fst t = item) 
     |> Option.bind (fun t -> Some (snd t))

   let add item count (Inventory inventory) =
     match countItem item (Inventory inventory) with
     |Some c -> create ((item,c + count)::(inventory |> List.filter (fun t -> fst t <> item)))
     |None -> create ((item,count)::inventory)

Inventory.create [{name="Sword of Death"},1;{name="Gold"},1]
  |> Inventory.add {name="Silver"} 1
  |> Inventory.add {name="Copper"} 5
  |> Inventory.add {name="Copper"} 5
  |> Inventory.add {name="Gold"} 2

Run in FSI the output is:

val it : Inventory = Inventory [({name = "Gold";}, 3); ({name = "Copper";}, 10); ({name = "Silver";}, 1); ({name = "Sword of Death";}, 1)]

Answer 2

There's a few small issues that I noticed with this.

The cons operator, ::, adds an item to the beginning of the list. I've done this in the Inventory.AddItem method. Since most people are used to items being added to the end of the list, the Inventory.AddItem method should be changed to this:

member this.AddItem(item: Item) =
    this.Items <- List.append [item] this.Items

Although, since List.append supports an entire 'T list as input, something like this can be done:

member this.AddItems(items: Item list) =
    this.Items <- List.append items this.Items

---

In addition to that, the Item.ChangeCount method is completely useless. I can just refactor Item.Count's setter to something like this:

member this.Count
    with get() = _count
    and set(value: int) =
        if this.Count + value >= 0 then
            this.Count <- this.Count + value
        else
            printfn "You cannot have less than zero items."