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

Hierarchical State Machine in F#

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

Problem

I appreciate any feedback on where I might alter or improve my code for this project. This is an old attempt at implementing a Hierarchical State Machine in F#. I'm from a C#/OO background mostly.

```
module FSharp.HSM

open Option
open System.Collections.Generic

exception NoTransition
exception NotInitialized
exception AlreadyStarted

type IStateMachine =
abstract member StateChanged: IEvent
abstract member Init: 'state -> unit
abstract member State: 'state with get
abstract member IsIn: 'state -> bool

abstract member Fire: 'event -> unit
abstract member Fire: 'event * obj -> unit

type Transition =
{ Event: 'event
NextState: 'event -> obj -> 'state option
Guard: unit -> bool }

type StateConfig =
{ State: 'state
Entry: unit -> unit
Exit: unit -> unit
SuperState: 'state option
Parents: 'state list
AutoTransition: 'state option
Transitions: Transition list }

type internal StateMachine(stateList:StateConfig list) =
let stateEvent = new Event()
let mutable current = stateList.Head.State
let mutable started = false
let states = stateList |> List.map (fun x -> x.State) |> List.toArray
let configs = new Dictionary>()
let find state : StateConfig = configs.[state]
let rec getParents results state =
let currentConfig = stateList |> List.find (fun x -> x.State = state)
if isSome currentConfig.SuperState then
let super = get currentConfig.SuperState
getParents (super::results) super
else results |> List.rev

do
for stateConfig in stateList do
configs.[stateConfig.State] List.tryFind (fun x -> x.Event = event), state.SuperState with
| None, None -> raise NoTransition
| None, Some x -> findTransition event (find x)
| Some x, _ -> x

let rec exitToCommonParent state limit =
match state.SuperState, limit with
| None, _ -> ()
| Some super, Some lim when super = lim -> ()
| Some super, _ ->

Solution

let rec getParents results state =
  let currentConfig = stateList |> List.find (fun x -> x.State = state)
  if isSome currentConfig.SuperState then 
    let super = get currentConfig.SuperState
    getParents (super::results) super
  else results |> List.rev


I think it's cleaner to use pattern matching here, it would avoid having to repeat currentConfig.SuperState.

Also, using accumulator like this makes sense if the results from the deepest level of recursion should be first, not last. But that's not what you want, so you can just drop the accumulator and then do something like super::(getParents super).

The rewritten function would look like this:

let rec getParents state =
  let currentConfig = stateList |> List.find (fun x -> x.State = state)
  match currentConfig.SuperState with
  | None -> []
  | Some super -> super::(getParents super)


It's possible you intentionally wrote it the way you did, because it's tail recursive, so it won't blow up your stack no matter how deep the recursion is. If that's the case, then I think you should make sure stack overflow is a real risk with the non-tail-recursive version. And then you should add a comment that explains that's the reason why you wrote it that way.

Code Snippets

let rec getParents results state =
  let currentConfig = stateList |> List.find (fun x -> x.State = state)
  if isSome currentConfig.SuperState then 
    let super = get currentConfig.SuperState
    getParents (super::results) super
  else results |> List.rev
let rec getParents state =
  let currentConfig = stateList |> List.find (fun x -> x.State = state)
  match currentConfig.SuperState with
  | None -> []
  | Some super -> super::(getParents super)

Context

StackExchange Code Review Q#43587, answer score: 4

Revisions (0)

No revisions yet.