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

Determining directions for a 2D boad

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

Problem

My original code is:

let (upPosition, downPosition, leftPosition, rightPosition) = 
    match myMove.MyDirection with
    | Direction.Up -> (GoUp y, y, GoLeft x, GoRight x)
    | Direction.Down-> (y, GoDown y, GoLeft x, GoRight x)
    | Direction.Left -> (GoUp y, GoDown y, GoLeft x, x)
    | Direction.Right -> (GoUp y, GoDown y, x, GoRight x)
    | _ -> (y, y, x, x)


Basically, there is an object on the 2D board. myMove.MyDirection dictates its next move, and x,y are its target position.

Once the object gets to x,y, it would cause some impacts on its front, left, right neighbours from its local rotational space. GoUp, ... , GoRight determines how far those neighbours could be.

So, ultimately, the code returns a "+" (cross) shape centred at x,y, so the minimum variable to describe this shape is (x, y, upPosition, downPosition, leftPosition, rightPosition).

The best solution I can think of to the previous code is the following. However, I feel there is more can be done to make it better. Is there any map function for tuple and is it costly?

let Reverse = function
    | Direction.Up -> Direction.Down
    | Direction.Down -> Direction.Up
    | Direction.Left -> Direction.Right
    | Direction.Right -> Direction.Left
    | _ -> raise (ArgumentException("Not Vaild Move!"))

let Go myDirection filter pos = 
    match myDirection, filter with
    | a,b when a=b -> pos
    | Direction.Up , _  -> GoUp pos
    | Direction.Down , _ -> GoDown pos
    | Direction.Left , _ -> GoLeft pos
    | Direction.Right , _ -> GoRight pos
    | _ -> pos

let (upPosition, downPosition, leftPosition, rightPosition) =
    let rev = Reverse myMove.MyDirection
    (Go Direction.Up rev y, Go Direction.Down rev y, Go Direction.Left rev x, Go Direction.Right rev x)

Solution

It's tough to review only part of the code, because there are often small improvements which could be made but they depend on knowing the structure of the entire program -- for example, why are you using a tuple to pass around the positions ((upPosition, downPosition, leftPosition, rightPosition))? You'd likely be better off using a record type to encapsulate those values, since you'd only have to pass one variable around (the record) and it also facilitates structural matching which'll allow you to clean up some of your other functions.

The Direction enum would be much better as a discriminated union type (see the code below). Even though enums can define certain values (e.g., Up, Down, Left, Right) they can also take on any integer value by casting the value to the enum type. Discriminated union types also allow you to define a fixed set of values, but the F# type system ensures that your variables can have only the defined values -- which in turn, keeps you from having to use the wildcard pattern (_) to check for error conditions in your match statements.

Here's my version of your code -- it's largely the same, but I've changed Direction into a union type and moved the Reverse function into a new Direction module.

type Direction =
    | Up
    | Down
    | Left
    | Right

[]
module Direction =
    /// Reverses a specified direction.
    []
    let rev = function
        | Direction.Up ->
            Direction.Down
        | Direction.Down ->
            Direction.Up
        | Direction.Left ->
            Direction.Right
        | Direction.Right ->
            Direction.Left

[]
let go myDirection filter pos =
    match myDirection with
    | x when x = filter ->
        pos
    | Direction.Up ->
        GoUp pos
    | Direction.Down ->
        GoDown pos
    | Direction.Left ->
        GoLeft pos
    | Direction.Right ->
        GoRight pos

let (upPosition, downPosition, leftPosition, rightPosition) =
    match myMove.MyDirection with
    | Direction.Up ->
        (GoUp y, y, GoLeft x, GoRight x)
    | Direction.Down->
        (y, GoDown y, GoLeft x, GoRight x)
    | Direction.Left ->
        (GoUp y, GoDown y, GoLeft x, x)
    | Direction.Right ->
        (GoUp y, GoDown y, x, GoRight x)

let (upPosition, downPosition, leftPosition, rightPosition) =
    let rev = Direction.rev myMove.MyDirection
    (go Direction.Up rev y, go Direction.Down rev y, go Direction.Left rev x, go Direction.Right rev x)


EDIT: Answers to your questions:

  • Normally, you can't have a module and a type with the same name within a certain namespace, but the [] attribute I used adds "Module" to the name of the compiled module type. So in this case, the Direction module would be compiled to a type called DirectionModule.



-
You could make rev a member function of the Direction type. However, in F# it's easier (in general) to consume module-based functions instead of type members because it works much better with F#'s type inference mechanism. In our example, if you had an instance of Direction you wouldn't be able to call myDirection.Reverse() unless you declared myDirection as a variable of type Direction (or the F# compiler was able to figure it out another way); if you call the module-based function (Direction.rev myDirection) F# can easily infer the type of the myDirection variable so you don't need any explicit type annotations.

If you're consuming this code from C#, then I'd write the code like this to get the best of both worlds:

type Direction =
    | Up
    | Down
    | Left
    | Right

    member this.Reverse () =
        match this with
        | Direction.Up ->
            Direction.Down
        | Direction.Down ->
            Direction.Up
        | Direction.Left ->
            Direction.Right
        | Direction.Right ->
            Direction.Left

[]
module internal Direction =
    /// Reverses a specified direction.
    []
    let inline rev (dir : Direction) =
        dir.Reverse ()


In this version of the code, I've defined Reverse as a member on the direction type (making it easy to consume from C#), but I also have the module-based function which is easier to consume from F#. Note the module-based function is now marked inline; as it only calls the Reverse() member it makes sense for the F# compiler to emit code which directly calls Reverse(). Another way to think about this is that within this version of the code, the module-based rev function is only useful for type-inference purposes (but it is useful if you're using it from within F#).

-
[] is an attribute specific to F#; when you use it on a function, the F# compiler will compile the function so it has the name you've specified in the attribute. This is primarily used when you want to consume some F# code from C#; you can define your F# functions with idiomatic F# names (e.g., rev), but by applying this attribute the methods in the compiled assembly can have idiomatic C# names (e.g., `

Code Snippets

type Direction =
    | Up
    | Down
    | Left
    | Right

[<RequireQualifiedAccess; CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
module Direction =
    /// Reverses a specified direction.
    [<CompiledName("Reverse")>]
    let rev = function
        | Direction.Up ->
            Direction.Down
        | Direction.Down ->
            Direction.Up
        | Direction.Left ->
            Direction.Right
        | Direction.Right ->
            Direction.Left


[<CompiledName("Go")>]
let go myDirection filter pos =
    match myDirection with
    | x when x = filter ->
        pos
    | Direction.Up ->
        GoUp pos
    | Direction.Down ->
        GoDown pos
    | Direction.Left ->
        GoLeft pos
    | Direction.Right ->
        GoRight pos

let (upPosition, downPosition, leftPosition, rightPosition) =
    match myMove.MyDirection with
    | Direction.Up ->
        (GoUp y, y, GoLeft x, GoRight x)
    | Direction.Down->
        (y, GoDown y, GoLeft x, GoRight x)
    | Direction.Left ->
        (GoUp y, GoDown y, GoLeft x, x)
    | Direction.Right ->
        (GoUp y, GoDown y, x, GoRight x)

let (upPosition, downPosition, leftPosition, rightPosition) =
    let rev = Direction.rev myMove.MyDirection
    (go Direction.Up rev y, go Direction.Down rev y, go Direction.Left rev x, go Direction.Right rev x)
type Direction =
    | Up
    | Down
    | Left
    | Right

    member this.Reverse () =
        match this with
        | Direction.Up ->
            Direction.Down
        | Direction.Down ->
            Direction.Up
        | Direction.Left ->
            Direction.Right
        | Direction.Right ->
            Direction.Left

[<RequireQualifiedAccess; CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
module internal Direction =
    /// Reverses a specified direction.
    [<CompiledName("Reverse")>]
    let inline rev (dir : Direction) =
        dir.Reverse ()

Context

StackExchange Code Review Q#15428, answer score: 3

Revisions (0)

No revisions yet.