5
\$\begingroup\$

I'm very new to functional world. I've written a simple brainfuck interpreter as my first F# program.

What I would like to know:

  • Am I using the right data structure for each situation?
  • Is my code easy to understand?
  • Is my code style similar to common F# codes (naming, organization etc)?
  • Is my code "very declarative" or I'm still thinking in the imperative way?

What is not so relevant:

  • Performance (but if I've made something very bad, let me now!)
  • More features

Obs.: I assume input is always a valid brainfuck code and that's ok to me.

open System
open System.IO

let getValue (position:int) (memory:Map<int, int>) =
    match Map.tryFind position memory with
    | None  -> 0
    | value -> value.Value

let calcAccumulator (acc:int) (instruction:char) (search:char) (miss:char) =
    match instruction with
    | x when x = search -> acc+1
    | x when x = miss   -> acc-1
    | _ -> acc

let rec findMatch (code:string) (search:char) (miss:char) (inc:int) (current:int) (acc:int) =
    let instruction = code.[current]

    match instruction, acc with
    | x, 0 when x = search -> current+1
    | _    -> findMatch code search miss inc (current+inc) (calcAccumulator acc instruction search miss)

let updateMemory (instruction:char) (position:int) (memory:Map<int, int>) =
    let oldValue = getValue position memory
    let newValue =
        match instruction with
        | '+' -> oldValue + 1
        | '-' -> oldValue - 1
        | ',' -> Console.ReadKey().KeyChar |> Convert.ToInt32
        | _   -> oldValue

    Map.add position newValue memory

let updateOutput (instruction) (value:int) =
    if instruction = '.'
    then Console.Write ((char) value)

let updatePosition (instruction:char) (position:int) =
    match instruction with
    | '>' -> position+1
    | '<' -> position-1
    | _   -> position

let updateIndex (code:string) (index:int) (value:int) =
    match code.[index], value with
    | '[', 0 -> findMatch code ']' '[' 1 (index+1) 0
    | ']', x when x <> 0 -> findMatch code '[' ']' -1 (index-1) 0
    | _      -> index+1

let rec interpretHelper (code:string) (index:int) (memory:Map<int, int>) (position:int) =
    match code.Length with
    | x when x = index  -> memory
    | _ ->
        let instruction = code.[index]
        let newPosition = updatePosition instruction position
        let newMemory = updateMemory instruction position memory
        let newIndex = updateIndex code index (getValue position memory)
        updateOutput instruction (getValue position memory)
        interpretHelper code newIndex newMemory newPosition

let interpret (code:string) =
    interpretHelper code 0 Map.empty 0

let options (args:string array) =
    let pathToFile = args.[0]
    let showMemory = 
        match args with
        | [| _ ; "-memory" |] -> true
        | _                   -> false

    pathToFile, showMemory

let onlyCode (text:string) =
    let validInstructions = "+-.,][<>".ToCharArray()

    String.filter (fun x -> Array.contains x validInstructions) text

[<EntryPoint>]
let main args =
    let pathToFile, showMemory = options args
    let code = onlyCode (IO.File.ReadAllText pathToFile)
    let memory = interpret code

    if showMemory
    then Map.iter (fun a b -> printf "\n%3d: %d" a b) memory

    0
\$\endgroup\$
  • \$\begingroup\$ What are you trying to achieve exactly? \$\endgroup\$ – THE AMAZING Apr 22 '16 at 14:12
  • \$\begingroup\$ @Richard_Grant Make the code easier to understand, more idiomatic and elegant. The goal is not performance, it's readability. \$\endgroup\$ – Gabriel Apr 22 '16 at 16:28
  • \$\begingroup\$ sorry, i don't understand the language. I am actually trying to learn from reading your code. Give me a few moments and i will throw out some suggestions. \$\endgroup\$ – THE AMAZING Apr 22 '16 at 16:32
  • 1
    \$\begingroup\$ Thank you. Sorry, I just didn't see your reply. Please, next time, use a nick in this way, for example: @Foggy Finder,... Then there will be a notification. \$\endgroup\$ – user110704 Jul 20 '16 at 14:17
  • 1
    \$\begingroup\$ I'll try to write an answer in 1-2 days. \$\endgroup\$ – user110704 Jul 20 '16 at 14:18
6
\$\begingroup\$

Let me make a few comments:

  1. F# has a good system of type inference. You don't need to specify the type explicitly, the compiler in many cases will do it for you. You can read more about it here.

  2. In function getValue. Usually, matched with None|Some value:

    match Map.tryFind position memory with
        | None  -> 0
        | Some value -> value
    

    Although you can rewrite this code with using defaultArg

    let getValue position memory =
        defaultArg (Map.tryFind position memory) 0 
    
  3. In function updateIndex you can check the value with 0 in match:

    let updateIndex (code:string) index value =
        match code.[index], value = 0 with
        | '[', true  -> findMatch code ']' '['  1 (index + 1) 0
        | ']', false -> findMatch code '[' ']' -1 (index - 1) 0
        | _      -> index + 1
    
  4. The helper functions better write as nested:

    let interpret (code:string) =
        let rec interpretHelper index memory position =
    

    You can read more about it here

  5. In function interpretHelper, use if-then-else instead of PM:

    let rec interpretHelper index memory position =
        if code |> String.length = index  then
            memory
        else ...
    
\$\endgroup\$

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.