7
\$\begingroup\$

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

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

    member this.Name = name
    member this.Count
        with get() = internalCount
        and set(value) = internalCount <- value

    /// <summary>
    /// Increment or decrement how many items there are.
    /// It will not allow for a negative count.
    /// </summary>
    /// <param name="amount">The amount to change the item count by.</param>
    member this.ChangeCount(amount) =
        if this.Count >= 1 then
            this.Count <- this.Count + amount

    override this.ToString() =
        String.Format("Name: \"{0}\", Count: \"{1}\"", this.Name, this.Count)


/// <summary>
/// This type represents an inventory, a collection
/// of values of the InventoryItem type.
/// </summary>
/// <param name="items">A list of InventoryItems.</param>
/// <param name="selectedItem">
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 <- value

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

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

    /// <summary>
    /// Change the currently selected item.
    /// </summary>
    /// <param name="amount">The amount to increase by.</param>
    member this.ChangeSelectedItem(amount:int) =
        let mutable itemCount = -1;
        for _ in this.Items do
            itemCount <- itemCount + 1

        if this.SelectedItem < itemCount && itemCount <> -1 then
            this.SelectedItem <- this.SelectedItem + amount

    override this.ToString() =
        String.Format(
            "Items: \"{0}\", Selected Item: \"{1}\"", 
            this.Items, 
            this.Items.[this.SelectedItem]
        )

Here's some example usage:

let myInventory = new Inventory([], 0)

myInventory.AddItem(new InventoryItem("Great Sword", 1))
myInventory.AddItem(new InventoryItem("Gold", 5))
myInventory.AddItem(new InventoryItem("Silver", 10))
myInventory.AddItem(new InventoryItem("Copper", 15))

Console.WriteLine(myInventory)

myInventory.ChangeSelectedItem(2)
Console.WriteLine(myInventory)

myInventory.ChangeSelectedItem(-1)
Console.WriteLine(myInventory)
\$\endgroup\$
7
\$\begingroup\$

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
\$\endgroup\$
  • \$\begingroup\$ I'm still fairly new to F#. What does the |> operator do? \$\endgroup\$ – Ethan Bierlein Jul 22 '15 at 15:46
  • 1
    \$\begingroup\$ @EthanBierlein - a |> b does b a. Due to some quirks of currying and type inference it is surprisingly useful. \$\endgroup\$ – John Palmer Jul 22 '15 at 23:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.