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

World's worst Christmas tree

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

Problem

Inspired by this question, I decided to grow my own fractal tree.

The problem is: given an integer \$n\$, \$0 \leq n \leq 5\$, print the \$n\$th iteration of the fractal tree. The tree is probably easiest to describe in pictures, so here's a minified version of the first iteration:

__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
__________________________________________________
_________________1_______________1________________
__________________1_____________1_________________
___________________1___________1__________________
____________________1_________1___________________
_____________________1_______1____________________
______________________1_____1_____________________
_______________________1___1______________________
________________________1_1_______________________
_________________________1________________________
_________________________1________________________
_________________________1________________________
_________________________1________________________
_________________________1________________________
_________________________1________________________
_________________________1________________________
_________________________1________________________


And the second iteration:

```
__________________________________________________
___________________________________________

Solution

Nice and clean code! There is nothing duplicated, and the functions are all small and clear. I like the different approach from the question that inspired you, taking on a more vector plotting solution, combining the lines afterwards. A few points (no pun intended) I could think of:

The abstraction of calling tree just leaks a bit, when you need to call the translateBy in the main function. You might want to either:

  • pass the columns parameter to the tree and centering the points there (which would move the recursive function to a sub-function tree')



Like this:

let tree n iterations columns =
    let rec tree' n iterations =
        match iterations with
        | 0 -> Seq.empty
        | _ -> seq {
                   yield! branch n
                   let child = tree' (n/2) (iterations - 1)
                   yield! child |> translateBy n (2*n)
                   yield! child |> translateBy (-n) (2*n)
               }
    tree' n iterations |> translateBy (columns/2 - 1) 0


  • or have a separate function called centerTree to do the translateBy part



Like this:

let centerTree columns points = points |> translateBy (columns/2 - 1) 0


Some statements switch between the "normal" parameter adding and the "forward pipe" |> style. I would replace code like

Seq.groupBy snd points
|> Seq.map


with code like

points
|> Seq.groupBy snd
|> Seq.map


just to keep the flow going.

When you avoid the for loop at the end, and replace it with a Seq.iter, that would be more "functional style", and you could write:

tree size iterations
|> centerTree columns
|> format rows columns
|> Seq.iter (printfn "%s")


which may or may not be your preferred style. Mind, the for loop is definitely idiomatic F#, it's just less FP, which I am just more fond of.

Code Snippets

let tree n iterations columns =
    let rec tree' n iterations =
        match iterations with
        | 0 -> Seq.empty
        | _ -> seq {
                   yield! branch n
                   let child = tree' (n/2) (iterations - 1)
                   yield! child |> translateBy n (2*n)
                   yield! child |> translateBy (-n) (2*n)
               }
    tree' n iterations |> translateBy (columns/2 - 1) 0
let centerTree columns points = points |> translateBy (columns/2 - 1) 0
Seq.groupBy snd points
|> Seq.map
points
|> Seq.groupBy snd
|> Seq.map
tree size iterations
|> centerTree columns
|> format rows columns
|> Seq.iter (printfn "%s")

Context

StackExchange Code Review Q#113338, answer score: 4

Revisions (0)

No revisions yet.