New concept exercise `dancing-dots` (`use` and `behaviour`) #1103

angelikatyborska · 2022-03-27T07:30:07Z

~~Heavily WIP!~~

~~I'm opening this PR to signal that I'm working on this.~~

This will be necessary as a prerequisite for the new GenServer exercise (#1076)

angelikatyborska · 2022-04-02T08:24:22Z

exercises/concept/dancing-dots/lib/dancing_dots/dot_group.ex

+  # This module is an example of how behaviours can be used in practice.
+  # You don't need to read it to solve this exercise.
+  # It's here for the curious :)


I'm not sure if it's a good idea to leave this code here. But one of my biggest problems with understanding behaviours is actually imagining how they can be used once I have some modules that all implement that one behaviour.

Yeah, I'm with you, it's nice to see how it can be used. render_dots/2 is great because it's very readable, but I think maybe new/2 is a bit too complex... Maybe it could be changed to filter the valid options rather than throwing an error for the sake of simplicity?

(untested idea)

def new(dots, animations_with_opts) do valid_animations_with_opts = for {animation_module, opts} <- animations_with_opts, # using Animation's init/1 callback {:ok, opts} <- animation_module.init(opts) do {animation_module, opts} end {:ok, %__MODULE__{dots: dots, animations_with_opts: valid_animations_with_opts }} end

I agree, new was too complex. I think the worst offender there was the reduce_while call. To make it simpler, I decided to split new into two functions - new that just returns a group with no animations, and add_animation to add a single animation module to the list. That allows getting rid of any Enum calls.

I didn't want to silently ignore initialization errors in this demo because that would show an anti-pattern IMO.

Yes, this new version is great!

angelikatyborska · 2022-04-02T08:27:58Z

@jiegillet The example solution, tests, and instructions are ready for review. Can you take a look?

jiegillet

Very very cool! The story is great and there is a lot to learn.

I took my time solving with only the instructions and the Elixir documentation. What a mess!! (the docs, not your instructions) All the ingredients you need for behaviours are scattered everywhere in like 3 or 4 different pages. This really is a needed exercise! I think even a nice blog post about the topic would be great.

For reference, here is my final submission, it's very very similar to yours:

defmodule DancingDots.Animation do
  @type dot :: DancingDots.Dot.t()
  @type opts :: keyword
  @type error :: any 
  @type frame_number :: pos_integer

  @callback init(opts) :: {:ok, opts} | {:error, error}
  defmacro __using__(_opts) do
    quote do
      @behaviour DancingDots.Animation
      def init(opts) do
        {:ok, opts}
      end 

      defoverridable init: 1
    end 
  end 

  @callback handle_frame(dot, frame_number, opts) :: dot 
end

defmodule DancingDots.Flicker do
  use DancingDots.Animation
  alias DancingDots.Dot

  @impl DancingDots.Animation
  def handle_frame(%Dot{opacity: opacity} = dot, frame_number, _opts)
      when rem(frame_number, 4) == 0 do
    %Dot{dot | opacity: opacity / 2}
  end 

  def handle_frame(%Dot{} = dot, _frame_number, _opts), do: dot 
end

defmodule DancingDots.Zoom do
  use DancingDots.Animation
  alias DancingDots.Dot

  @impl DancingDots.Animation
  def init(opts) do
    velocity = Keyword.get(opts, :velocity)

    if is_number(velocity) do
      {:ok, opts}
    else
      {:error, "Expected required option :velocity to be a number, got: #{inspect(velocity)}"}
    end 
  end 

  @impl DancingDots.Animation
  def handle_frame(%Dot{radius: radius} = dot, frame_number, opts) do
    n = frame_number - 1 
    velocity = Keyword.get(opts, :velocity)
    %Dot{dot | radius: radius + n * velocity}
  end 
end

Outside of finding information in the docs, the only place that gave me pause was the error message in DancingDots.Zoom, I left some comments about that.

exercises/concept/dancing-dots/.docs/instructions.md

exercises/concept/dancing-dots/lib/dancing_dots/dot_group.ex

jiegillet · 2022-04-02T12:08:50Z

exercises/concept/dancing-dots/lib/dancing_dots/dot_group.ex

+  # This module is an example of how behaviours can be used in practice.
+  # You don't need to read it to solve this exercise.
+  # It's here for the curious :)


Yeah, I'm with you, it's nice to see how it can be used. render_dots/2 is great because it's very readable, but I think maybe new/2 is a bit too complex... Maybe it could be changed to filter the valid options rather than throwing an error for the sake of simplicity?

(untested idea)

def new(dots, animations_with_opts) do valid_animations_with_opts = for {animation_module, opts} <- animations_with_opts, # using Animation's init/1 callback {:ok, opts} <- animation_module.init(opts) do {animation_module, opts} end {:ok, %__MODULE__{dots: dots, animations_with_opts: valid_animations_with_opts }} end

exercises/concept/dancing-dots/.meta/design.md

jiegillet · 2022-04-02T12:17:48Z

exercises/concept/dancing-dots/.meta/exemplar.ex

+    if is_number(velocity) do
+      {:ok, [velocity: velocity]}
+    else
+      {:error, "Expected required option :velocity to be a number, got: #{inspect(velocity)}"}


I would suggest to add one type of errors here: "Missing option :velocity". And some tests for that.

I'm not sure I understand the suggestion, are you saying there need to be two different errors, one for when velocity was not given, and one for when it's given but not valid? There already is a test for when it's missing:

assert DancingDots.Zoom.init([]) == {:error, "Expected required option :velocity to be a number, got: nil"}

Yes, I was suggesting two different errors, but actually a simple rephrasing might do.

If no :velocity option is given, the error message says "Expected required option :velocity to be a number", which feels like you passed something wrong, not nothing at all.

How about:
"The :velocity option is required, and its value must be a number. Got: nil"
It's a small change, but it feels a bit better maybe?

Totally, that does sound better. Here: ccfcabe (#1103)

exercises/concept/dancing-dots/test/dancing_dots/animation_test.exs

github-actions · 2022-04-08T09:08:51Z

Thank you for contributing to exercism/elixir 💜 🎉. This is an automated PR comment 🤖 for the maintainers of this repository that helps with the PR review process. You can safely ignore it and wait for a maintainer to review your changes.

Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:

General steps
- 🏆 Does this PR need to receive a label with a reputation modifier (x:size/{tiny,small,medium,large,massive})? (A medium reputation amount is awarded by default, see docs)
Any exercise changed
- 👤 Does the author of the PR need to be added as an author or contributor in <exercise>/.meta/config.json (see docs)?
- 🔬 Do the analyzer and the analyzer comments exist for this exercise? Do they need to be changed?
- 📜 Does the design file (<exercise>/.meta/design.md) need to be updated to document new implementation decisions?
Concept exercise changed
- 🌲 Do prerequisites and practices in config.json need to be updated?
- 📖 Does the concept introduction provide all necessary information to solve this exercise?
Concept exercise tests changed
- ⚪️ Are all tests un-skipped?
- 🔢 Are all tests annotated with @tag task_id?
- 🐈 Can all tests be understood by reading the test's block only (e.g. no module attributes, no setup functions)?
Concept changed
- 👤 Does the author of the PR need to be added as an author or contributor in <concept>/.meta/config.json (see docs)?
- 📖 Does the concept introduction provide all necessary information to solve this exercise?

Automated comment created by PR Commenter 🤖.

concepts/behaviours/about.md

concepts/behaviours/introduction.md

concepts/use/about.md

concepts/use/introduction.md

angelikatyborska · 2022-04-08T13:29:48Z

This is now ready for the final full review.

exercises/concept/dancing-dots/.docs/hints.md

exercises/concept/dancing-dots/.docs/instructions.md

exercises/concept/dancing-dots/.docs/introduction.md

Co-authored-by: Victor Goff <[email protected]>

jiegillet

Awesome work 🎉
I left some comments, but this is already good enough to merge.

concepts/behaviours/.meta/config.json

exercises/concept/dancing-dots/.docs/introduction.md

jiegillet · 2022-04-10T13:33:09Z

exercises/concept/dancing-dots/.meta/design.md

+- `structs`
+- `ast`
+- `enum`
+- `import`


Is there a need for an analyzer as well?

The only thing I can thing of is that DancingDots.Zoom.init/1 uses Keyword and is_number but it's very minor.

I'm not sure if those two things even matter, using the access behaviour (opts[:velocity]) is also fine. I guess the only "bad idea" that technically works would be pattern matching on the keyword list. As for is_number, we could add more tests that would assert that it also allows floats or something, but I don't think that's really necessary.

I could think of two things, relevant to the concept:

362ceec (#1103)

Those are better, yes :)

jiegillet · 2022-04-10T13:38:22Z

exercises/concept/dancing-dots/lib/dancing_dots/dot_group.ex

+  # This module is an example of how behaviours can be used in practice.
+  # You don't need to read it to solve this exercise.
+  # It's here for the curious :)


Yes, this new version is great!

Co-authored-by: Jie <[email protected]>

angelikatyborska added 10 commits March 27, 2022 09:27

Solution first draft

43d2f23

Update config

a1a11e7

Write tests

98c4faa

Remove unnecessary code

ff94fa6

Write instructions

c03269d

Bring back unnecessary demo code

a89a48c

Integrate demo code into test

7d04120

Provide boilerplate solution

0ba3a0e

Rephrase instructions

11dafd0

Leave notes in intro for later

c4575bf

angelikatyborska commented Apr 2, 2022

View reviewed changes

angelikatyborska added 2 commits April 2, 2022 10:24

Fix module name in mix.exs

704bb39

Fix indentation

0f70635

angelikatyborska added the x:size/large Large amount of work label Apr 2, 2022

jiegillet reviewed Apr 2, 2022

View reviewed changes

angelikatyborska mentioned this pull request Apr 2, 2022

New concept exercise: take-a-number-deluxe (genserver) #1076

Merged

angelikatyborska added 6 commits April 8, 2022 10:20

Fill out 'out of scope'

00c6003

Split zoom test into pos and neg velocity

94c3fd6

Simplify dot group

b15b088

Bring back comment

77d19bd

Fill out config

c7122f1

Create concept directories

d5b6d46

angelikatyborska added 5 commits April 8, 2022 11:10

Mention velocity error in instructions

2099679

Run configlet format

6ab5812

Write use intro

3f21f43

Write a behaviours intro

faff1bf

Fill out blurbs

1aef4f9

angelikatyborska commented Apr 8, 2022

View reviewed changes

concepts/behaviours/about.md Outdated Show resolved Hide resolved

angelikatyborska commented Apr 8, 2022

View reviewed changes

concepts/behaviours/introduction.md Outdated Show resolved Hide resolved

angelikatyborska commented Apr 8, 2022

View reviewed changes

concepts/use/about.md Outdated Show resolved Hide resolved

angelikatyborska commented Apr 8, 2022

View reviewed changes

concepts/use/introduction.md Outdated Show resolved Hide resolved

Write hints

0eea950

angelikatyborska marked this pull request as ready for review April 8, 2022 13:29

kotp reviewed Apr 8, 2022

View reviewed changes

angelikatyborska and others added 3 commits April 9, 2022 10:09

Fix spelling

5827335

Update exercises/concept/dancing-dots/.docs/instructions.md

39dee25

Co-authored-by: Victor Goff <[email protected]>

Update exercises/concept/dancing-dots/.docs/instructions.md

def7439

Co-authored-by: Victor Goff <[email protected]>

jiegillet approved these changes Apr 10, 2022

View reviewed changes

angelikatyborska and others added 7 commits April 10, 2022 16:09

Improve velocity error message

ccfcabe

Update concepts/behaviours/.meta/config.json

eebe603

Co-authored-by: Jie <[email protected]>

Update exercises/concept/dancing-dots/.docs/introduction.md

6f789a2

Co-authored-by: Jie <[email protected]>

Add default callback implementation example

5e79019

Add analyzer hints

362ceec

Copy-paste intros and abouts

57f4ca5

Add jie as contributor to concepts

44852a5

angelikatyborska merged commit d4de16d into main Apr 10, 2022

angelikatyborska deleted the new-concept-exercise-behaviours branch April 10, 2022 14:47

This was referenced Jul 14, 2022

Document reputation label exercism/docs#347

Merged

Consider correct reputation amounts for different labels exercism/exercism#6440

Open

Consider correct reputation amounts for different labels exercism/exercism#6441

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New concept exercise `dancing-dots` (`use` and `behaviour`) #1103

New concept exercise `dancing-dots` (`use` and `behaviour`) #1103

angelikatyborska commented Mar 27, 2022 •

edited

Loading

angelikatyborska Apr 2, 2022

jiegillet Apr 2, 2022

angelikatyborska Apr 8, 2022

jiegillet Apr 10, 2022

angelikatyborska commented Apr 2, 2022

jiegillet left a comment

jiegillet Apr 2, 2022

jiegillet Apr 2, 2022

angelikatyborska Apr 8, 2022

jiegillet Apr 10, 2022

angelikatyborska Apr 10, 2022

github-actions bot commented Apr 8, 2022

angelikatyborska commented Apr 8, 2022

jiegillet left a comment

jiegillet Apr 10, 2022

angelikatyborska Apr 10, 2022

jiegillet Apr 10, 2022

jiegillet Apr 10, 2022

New concept exercise dancing-dots (use and behaviour) #1103

New concept exercise dancing-dots (use and behaviour) #1103

Conversation

angelikatyborska commented Mar 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angelikatyborska commented Apr 2, 2022

jiegillet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 8, 2022

angelikatyborska commented Apr 8, 2022

jiegillet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

New concept exercise `dancing-dots` (`use` and `behaviour`) #1103

New concept exercise `dancing-dots` (`use` and `behaviour`) #1103

angelikatyborska commented Mar 27, 2022 •

edited

Loading