A possible refactoring of the example code?

[pragdave]

First, congratulations on an exceptional achievement. Nicely done.

These days I'm on a mission to create Elixir code with smaller chunks of things, where a chunk could be a module, function, or even expression. Looking through this code, it seems like you're already there.

So I have a small suggestion for the example code generated by new.scenic.example.

Take the primitives code as an example. Right now, it looks like this:

defmodule Xx.Scene.Primitives do
  @moduledoc """
  Sample scene.
  """

  use Scenic.Scene
  alias Scenic.Graph
  import Scenic.Primitives

  alias Xx.Component.Nav
  alias Xx.Component.Notes

  @bird_path :code.priv_dir(:xx)
             |> Path.join("/static/images/cyanoramphus_zealandicus_1849.jpg")
  @bird_hash Scenic.Cache.Hash.file!( @bird_path, :sha )

  @bird_width 100
  @bird_height 128

  @body_offset 80

  @line {{0, 0}, {60, 60}}

  @notes """
    \"Primitives\" shows the various primitives available in Scenic.
    It also shows a sampling of the styles you can apply to them.
  """

  @graph Graph.build(font: :roboto, font_size: 24)
         |> group(
           fn g ->
             g
             |> text("Various primitives and styles", translate: {15, 20})

             # lines
             |> group(
               fn g ->
                 g
                 |> line(@line, stroke: {4, :red})
                 |> line(@line, stroke: {20, :green}, cap: :butt, t: {60, 0})
                 |> line(@line, stroke: {20, :yellow}, cap: :round, t: {120, 0})
               end,
               t: {290, 50}
             )

             # row
             |> group(
               fn g ->
                 g
                 |> triangle({{0, 60}, {50, 0}, {50, 60}}, fill: :khaki, stroke: {4, :green})
                 |> circle(30, fill: :green, stroke: {6, :yellow}, t: {110, 30})
                 |> ellipse({30, 40}, rotate: 0.5, fill: :green, stroke: {4, :gray}, t: {200, 30})
               end,
               t: {15, 50}
             )

             # row
             |> group(
               fn g ->
                 g
                 |> rect({50, 60}, fill: :khaki, stroke: {4, :green})
                 |> rrect({50, 60, 6}, fill: :green, stroke: {6, :yellow}, t: {85, 0})
                 |> quad({{0, 100}, {160, 0}, {300, 110}, {200, 260}},
                   id: :quad,
                   fill: {:linear, {0, 0, 400, 400, :yellow, :purple}},
                   stroke: {10, :khaki},
                   translate: {160, 0},
                   scale: 0.3,
                   pin: {0, 0}
                 )
                 |> sector({100, -0.3, -0.8},
                   stroke: {3, :grey},
                   fill: {:radial, {0, 0, 20, 160, {:yellow, 128}, {:purple, 128}}},
                   translate: {270, 70}
                 )
                 |> arc({100, -0.1, -0.8}, stroke: {6, :orange}, translate: {320, 70})
               end,
               t: {15, 130}
             )

             # bird
             |> rect({@bird_width, @bird_height}, fill: {:image, @bird_hash}, t: {15, 230})

             # text
             |> group(
               fn g ->
                 g
                 |> text("Hello", translate: {0, 40})
                 |> text("World", translate: {90, 40}, fill: :yellow)
                 |> text("Blur", translate: {0, 80}, font_blur: 2)
                 |> text("Shadow", translate: {82, 82}, font_blur: 2, fill: :light_grey)
                 |> text("Shadow", translate: {80, 80})
               end,
               font_size: 40,
               t: {130, 240}
             )

             # twisty path
             |> path(
               [
                 :begin,
                 {:move_to, 0, 0},
                 {:bezier_to, 0, 20, 0, 50, 40, 50},
                 {:bezier_to, 60, 50, 60, 20, 80, 20},
                 {:bezier_to, 100, 20, 110, 0, 120, 0},
                 {:bezier_to, 140, 0, 160, 30, 160, 50}
               ],
               stroke: {2, :red},
               translate: {355, 230},
               rotate: 0.5
             )
           end,
           translate: {15, @body_offset}
         )

         # Nav and Notes are added last so that they draw on top
         |> Nav.add_to_graph(__MODULE__)
         |> Notes.add_to_graph(@notes)

  def init(_, _opts) do
    # load the parrot texture into the cache
    Scenic.Cache.File.load(@bird_path, @bird_hash)

    push_graph(@graph)

    {:ok, @graph}
  end
end

The @graph attribute is scary long, and I constantly messed up brackets when messing with it. So how about continuing the compile-time attribute goodness by splitting it further?

defmodule Me1.Scene.Primitives do
  @moduledoc """
  Sample scene.
  """

  use Scenic.Scene
  alias Scenic.Graph
  import Scenic.Primitives

  alias Me1.Component.Nav
  alias Me1.Component.Notes

  @bird_path :code.priv_dir(:me1)
             |> Path.join("/static/images/cyanoramphus_zealandicus_1849.jpg")
  @bird_hash Scenic.Cache.Hash.file!( @bird_path, :sha )

  @bird_width 100
  @bird_height 128

  @body_offset 80

  @line {{0, 0}, {60, 60}}

  @notes """
    \"Primitives\" shows the various primitives available in Scenic.
    It also shows a sampling of the styles you can apply to them.
  """

  @lines fn g ->
    g
    |> line(@line, stroke: {4, :red})
    |> line(@line, stroke: {20, :green}, cap: :butt, t: {60, 0})
    |> line(@line, stroke: {20, :yellow}, cap: :round, t: {120, 0})
  end

  @triangle_circle_ellipse fn g ->
    g
    |> triangle({{0, 60}, {50, 0}, {50, 60}}, fill: :khaki, stroke: {4, :green})
    |> circle(30, fill: :green, stroke: {6, :yellow}, t: {110, 30})
    |> ellipse({30, 40}, rotate: 0.5, fill: :green, stroke: {4, :gray}, t: {200, 30})
  end

  # or possibly using & function literals??

  @filled_stuff & &1
    |> rect({50, 60}, fill: :khaki, stroke: {4, :green})
    |> rrect({50, 60, 6}, fill: :green, stroke: {6, :yellow}, t: {85, 0})
    |> quad({{0, 100}, {160, 0}, {300, 110}, {200, 260}},
      id: :quad,
      fill: {:linear, {0, 0, 400, 400, :yellow, :purple}},
      stroke: {10, :khaki},
      translate: {160, 0},
      scale: 0.3,
      pin: {0, 0}
    )
    |> sector({100, -0.3, -0.8},
      stroke: {3, :grey},
      fill: {:radial, {0, 0, 20, 160, {:yellow, 128}, {:purple, 128}}},
      translate: {270, 70}
    )
    |> arc({100, -0.1, -0.8}, stroke: {6, :orange}, translate: {320, 70})


  @text & &1
    |> text("Hello", translate: {0, 40})
    |> text("World", translate: {90, 40}, fill: :yellow)
    |> text("Blur", translate: {0, 80}, font_blur: 2)
    |> text("Shadow", translate: {82, 82}, font_blur: 2, fill: :light_grey)
    |> text("Shadow", translate: {80, 80})


  @path &path(&1,
      [
        :begin,
        {:move_to, 0, 0},
        {:bezier_to, 0, 20, 0, 50, 40, 50},
        {:bezier_to, 60, 50, 60, 20, 80, 20},
        {:bezier_to, 100, 20, 110, 0, 120, 0},
        {:bezier_to, 140, 0, 160, 30, 160, 50}
      ],
      stroke: {2, :red},
      translate: {355, 230},
      rotate: 0.5)

  @graph Graph.build(font: :roboto, font_size: 24)
         |> group(
           fn g ->
             g
             |> text("Various primitives and styles", translate: {15, 20})
             |> group(@lines, t: {290, 50})
             |> group(@triangle_circle_ellipse, t: {15, 50})
             |> group(@filled_stuff, t: {15, 130})
             |> rect({@bird_width, @bird_height}, fill: {:image, @bird_hash}, t: {15, 230})
             |> group(@text, font_size: 40, t: {130, 240})
             |> group(@path)
           end,
           translate: {15, @body_offset}
         )

         # Nav and Notes are added last so that they draw on top
         |> Nav.add_to_graph(__MODULE__)
         |> Notes.add_to_graph(@notes)

  def init(_, _opts) do
    # load the parrot texture into the cache
    Scenic.Cache.File.load(@bird_path, @bird_hash)

    push_graph(@graph)

    {:ok, @graph}
  end
end

You can see I switched styles from using fn to using & half way through just to see what each looked like.

Waddya think?

Dave

(ps: I think I get major credit for not showing you my solution using macros... :)

[boydm]

Yes. I like that.

This is one those cases where it looked much cleaner before running "mix format".

Breaking up the graph into pre-compiled sub-chunks-things is definitely a good best practice. So yes. That will happen.

I'm mixed on the & vs fn formatting. I prefer the look of the & version, but it isn't intuitive for a beginner as to what is going on. I can just imagine a beginner getting to a line that reads @text & &1 and giving up. Wish there was a compact version that looked like it had less magic. Open to suggestions.

I too thought about writing a DSL and macros and all that. Want to keep it as simple as possible until there is a burning need to add magic to make it cleaner. Encouraging a best practice of breaking up the code is a great way to avoid that scenario.

But most of all - Thank you for the kind words!

[thovoll]

Thanks for this @pragdave, it helps a lot!
@boydm: I'm an Elixir beginner and I'm definitely more comfortable with fn over &.

[pragdave]

Do you want a PR?

…

On Sun, Nov 18, 2018, 07:13 Thomas Vollmer ***@***.*** wrote: Thanks for this @pragdave <https://github.com/pragdave>, it helps a lot! @boydm <https://github.com/boydm>: I'm an Elixir beginner and I'm definitely more comfortable with fn over &. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#113 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAApmMuHihOiSHB1pPVysUCceCNChLlZks5uwVzogaJpZM4YnwSX> .

[Eiji7]

I'm mixed on the & vs fn formatting. I prefer the look of the & version, but it isn't intuitive for a beginner as to what is going on. I can just imagine a beginner getting to a line that reads @text & &1 and giving up. Wish there was a compact version that looked like it had less magic. Open to suggestions.

This example is really bad even for non-beginners. It says almost nothing. I would add a prefix and suffix which should say much more for example: graph_text_func = & &1, so even beginner can see func part and would start searching for explanation like Elixir function references or Elixir function short version which I think should be enough. Also graph part is important, because it could say where it would be used in this file. Finally I would like to remove @ character making it compile time variable as there is no need to use module attribute which would not be called from any function, so only final @graph should be an attribute.

What do you think about it?

[boydm]

Do you want a PR?

Yes please Dave. I'm up to my eyeballs in features for 0.10.0, services to support it and dealing with the fact that I just moved halfway around the planet. (New Zealand is very nice :)

I'm sure you can turn this into a teaching moment with regarding the whole fn vs & thing.

[pragdave]

While I'm on a roll, how about one wafer-thin addition to the API that curries the various primitives so they can be treated as data:

  @lines [
    line_spec(@line, stroke: {4, :red}),
    line_spec(@line, stroke: {20, :green}, cap: :butt, t: {60, 0}),
    line_spec(@line, stroke: {20, :yellow}, cap: :round, t: {120, 0}),
  ]

  @triangle_circle_ellipse [
    triangle_spec({{0, 60}, {50, 0}, {50, 60}}, fill: :khaki, stroke: {4, :green}),
    circle_spec(30, fill: :green, stroke: {6, :yellow}, t: {110, 30}),
    ellipse_spec({30, 40}, rotate: 0.5, fill: :green, stroke: {4, :gray}, t: {200, 30}),
  ]


  @text [
    text_spec("Hello", translate: {0, 40}),
    text_spec("World", translate: {90, 40}, fill: :yellow),
    text_spec("Blur", translate: {0, 80}, font_blur: 2),
    text_spec("Shadow", translate: {82, 82}, font_blur: 2, fill: :light_grey),
    text_spec("Shadow", translate: {80, 80}),
  ]

  @graph Graph.build(font: :roboto, font_size: 24)
         |> mgroup([
             text_spec("Various primitives and styles", translate: {15, 20}),
             group_spec(@lines, t: {290, 50}),
             group_spec(@triangle_circle_ellipse, t: {15, 50}),
             group_spec(@text, font_size: 40, t: {130, 240}),
           ],
           translate: {15, @body_offset}
         )

         # Nav and Notes are added last so that they draw on top
         |> Nav.add_to_graph(__MODULE__)
         |> Notes.add_to_graph(@notes)

The additions to primitive.ex are simple. Again, I'd be happy to generate a PR.

(and I'm not wedded to the name xxx_spec. Any suggestions welcome.)

 def line_spec(points, opts) do
    fn g -> line(g, points, opts) end
  end

  def triangle_spec(points, opts) do
    fn g -> triangle(g, points, opts) end
  end


  def circle_spec(radius, opts) do
    fn g -> circle(g, radius, opts) end
  end

  def ellipse_spec(radii, opts) do
    fn g -> ellipse(g, radii, opts) end
  end

  def text_spec(text, opts) do
    fn g -> text(g, text, opts) end
  end

  def group_spec(list, opts) when is_list(list) do
    fn g ->
      content = fn g ->
        Enum.reduce(list, g, fn  element, g  -> element.(g) end)
      end
      group(g, content, opts)
    end
  end

  def group_spec(item, opts) do
    group_spec([item], opts)
  end

  def mgroup(g, list, opts \\ []) do
    display_list = fn g ->
      list
       |> Enum.reduce(g, fn item, g -> item.(g) end)
    end
    group(g, display_list, opts)
  end

[boydm]

This is similar in spirit to what I was trying to do with the primitives.ex helper functions. I don't have an opinion yet. Just some thoughts.

pros: Very clean to read as it looks more data-like. Hides the compile time function calls. Visually gets rid of both the fn and & forms and avoids that whole confusion.

cons: Makes it less clear that the primitives are being added to a graph. It loses the pipelining, which makes it obvious that a graph is being transformed with each call.

This is less clear to me. Is there enough gained in cleaning it up visually to make up for the added "magic" of hiding the fact that it is transforming a graph? The end result is the same either way. The graph is transformed and that is the final object compiled and stored into the object code.

This worry is somewhat mitigated by the mgroup function, who's job is to accept the data format and treat it as if was the pipeline form. However, it adds a new call that effectively means there are mixed metaphors in the code.

What do you (and others) think?

[pragdave]

How about renaming mgroup to add_to_graph?

…

On Sun, Nov 18, 2018 at 3:15 PM Boyd Multerer ***@***.***> wrote: This is similar in spirit to what I was trying to do with the primitives.ex helper functions. I don't have an opinion yet. Just some thoughts. pros: Very clean to read as it looks more data-like. Hides the compile time function calls. Visually gets rid of both the fn and & forms and avoids that whole confusion. cons: Makes it less clear that the primitives are being added to a graph. It loses the pipelining, which makes it obvious that a graph is being transformed with each call. This is less clear to me. Is there enough gained in cleaning it up visually to make up for the added "magic" of hiding the fact that it is transforming a graph? The end result is the same either way. The graph is transformed and that is the final object compiled and stored into the object code. This worry is somewhat mitigated by the mgroup function, who's job is to accept the data format and treat it as if was the pipeline form. However, it adds a new call that effectively means there are mixed metaphors in the code. What do you (and others) think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#113 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAApmAxfTZ2WxmjbljM5XYcwKppGWaYcks5uwc3ZgaJpZM4YnwSX> .

[boydm]

add_to_graph at is definitely better.

Go for it.

[boydm]

I'm in the process of bringing this in to Scenic. Should go out in v0.10.0 next week

[pragdave]

On, I'm sorry. I totally missed that is said if so that. I can still do it if you haven't started.

[boydm]

Hi Dave! If you have time to refactor more of the sample code, that would be great. I'm trying to bring in a big font metrics thing I've been working on. TrueType is a total beast. Will be nice to have tho.

Otherwise, v0.10 will go out with some scenes done the spec way, some the the pipeline way. I don't mind showing both.

[pragdave]

Boyd: I went ahead and did that, then bumped into the issue that I hadn’t created xxx_spec functions for components. I looked into it and it seems like it _should_ word by simply currying the existing component functions, so for def button(%Graph{} = g, data, options) do add_to_graph(g, Component.Button, data, options) end I'd also implement def button_spec(data, options) do fn g -> button(g, data, options) end end But before hacking I thought I’d check that this was reasonable. Dave

[boydm]

That is reasonable and sounds good to me. Makes me happy in fact that the functions are working as intended!

Makes me think tho that, unlike the primitives, others should feel encouraged to make new components and make them public. Will need to document guidance on how they can have their work fit the pattern...

[pragdave]

That is reasonable and sounds good to me. Makes me happy in fact that the functions are working as intended! OK, I’ll get you a PR that should work with 0.10.0 Makes me think tho that, unlike the primitives, others should feel encouraged to make new components and make them public. Will need to document guidance on how they can have their work fit the pattern… I think that’s a valid point. However, as I play around with the _spec stuff I’m noticing patterns in what I do: in particular I sometimes need to modify elements to (for example) center them in the viewport. That’s obviously doable, but ~~~ elixir @graph |> Graph.modify(:temperature, &update_opts(&1, translate: {vp_width / 2, @font_size})) |> Graph.modify(:button_1, &update_opts(&1, width: col*4)) |> Graph.modify(:button_2, &update_opts(&1, width: col*2 - 6)) |> Graph.modify(:button_3, &update_opts(&1, width: col*2 - 6), translate: {col * 2 + 6, 60}) |> push_graph() ~~~ Feels too clunky. So I’m not convinced we’re reached the end of this road yet. Dave

[boydm]

There is other post v0.10 work that will alleviate some of that. Namely, a responsive group, where you can specify columns and it will adjust the content (via transforms) to get the positions right. Should work across multiple screen sizes.

Also, the font_metrics I'm working on now mean that buttons can optionally resize themselves to fit the content and have better vertical centering. All the components will see some level of improvement from that work.

[boydm]

This is all in master. Going out in v.10