Leap 48in24 approaches #1401

michalporeba · 2024-01-06T21:23:18Z

This resolves #1400

I propose an introduction to the approaches and specific approaches to the problem.
Considering the time until 48in24 I welcome early feedback on the text and the selection of the approaches.
I will work on specific approaches in subsequent commits.

The difference between rem/2 and Integer.mod/2 might be an interesting article to enrich the deep dive, however, due to time constraints and the need to write up approaches in other languages, I consider it out of scope for this PR.

github-actions · 2024-01-06T21:23:31Z

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?
Practice exercise changed
- 🌲 Do prerequisites, practices, and difficulty in config.json need to be updated?
- 🧑‍🏫 Are the changes in accordance with the community-wide problem specifiations?

Automated comment created by PR Commenter 🤖.

exercises/practice/leap/.approaches/case/snippet.txt

exercises/practice/leap/.approaches/cond/snippet.txt

Tuxified · 2024-01-08T15:47:31Z

This looks great, I'm not sure if explaining the difference between Kernel.rem/2 and Integer.mod/2 will be that interesting for the purpose of this challenge. Besides these approaches we could also add an if expression (translated from a Go solution):

def leap?(year) do
  if rem(year, 100) == 0 do
    rem(year, 400) == 0
  else
    rem(year, 4) == 0
  end
end

Really super small pet peeve: Elixir is usually indented using 2 spaces, not 4 ;)

angelikatyborska · 2024-01-08T17:40:32Z

I was holding off any reviews on this as it's still marked a draft, but let me just address:

Really super small pet peeve: Elixir is usually indented using 2 spaces, not 4 ;)

I'm unhappy that the snippets have to be .txt files (as I learned on Discord the configuration option for the file format doesn't work, not your fault...) Before approving this PR, I intend to write some small script that will turn them to .ex, autoformat, and then turn back.

angelikatyborska · 2024-01-08T20:30:12Z

@michalporeba I pushed a commit with a script bin/format_approach_snippets.sh that you can run to format the code snippet. I couldn't find anything that could also format Elixir code in markdown files, so we will need to make sure manually that they are formatted correctly too.

michalporeba · 2024-01-08T20:48:55Z

Thanks for the feedback. Please don't wait. There is little time and there is still a lot to do. This is why I created a draft PR to ask for early feedback. I still have specific approaches to write up, and the snippets may change slightly once the approaches are written up.

At the moment, I'm mostly interested in what you think about the structure of the introduction selections of the approaches. I'll start on the individual approaches tomorrow.

michalporeba · 2024-01-09T11:03:56Z

@michalporeba I pushed a commit with a script bin/format_approach_snippets.sh that you can run to format the code snippet. I couldn't find anything that could also format Elixir code in markdown files, so we will need to make sure manually that they are formatted correctly too.

Hi @angelikatyborska. The scrip appears to be causing issues with checks. Considering that there will be more code in the .md files in the discussion than there will be in the snippets, perhaps we need to depend on manual code formatting?

Snippets also often contain partial code, that's my experience from other tracks. The idea is to illustrate briefly what will be discussed on the approach page. Won't that be a problem for the formatted?

angelikatyborska · 2024-01-09T15:29:49Z

That's on purpose, I want CI checks to fail if the code in snippets is not formatted. This will also require that the snippets are formally valid Elixir code, but it doesn't need to actually compile.

It would be nice if I could do the same for the code in .md files, but for now we need to manually format there.

angelikatyborska

Thanks for working on this! I personally don't have the motivation to write any approaches myself, but if do, I promise to review all your PRs on time 😇.

For the future it's better to tell me explicitly that you want a draft PR reviewed when you open it. I usually wait until it's marked as "ready for review" because, well... that's the name of the feature 😂 and some people are shy about having their work in progress judged.

exercises/practice/leap/.approaches/introduction.md

angelikatyborska · 2024-01-10T16:51:12Z

exercises/practice/leap/.approaches/introduction.md

+The two functions differ in how they work with negative numbers, but since, in this exercise,
+all the numbers are non-negative, both could work, depending on the approach.


Nitpick: I think there are no test cases for the year 0, so we could also say:

Suggested change

The two functions differ in how they work with negative numbers, but since, in this exercise,

all the numbers are non-negative, both could work, depending on the approach.

The two functions differ in how they work with negative numbers, but since, in this exercise,

all the input numbers are positive, both could work, depending on the approach.

(I'm specifying "input numbers" instead of just "numbers" to avoid people complaining that technically there is a zero in this exercise, in rem(year, n) == 0 😛 )

I used the "non-negative" here very intentionally. Positives start at 1, non-negative start at 0, so the zero is included. Probably a better name for the n parameter and for the description would be to say 'divisor' as this is technically what it is, and where the behaviour of the two functions differ.

alright, no problem, we can keep it

exercises/practice/leap/.approaches/introduction.md

angelikatyborska · 2024-01-10T17:26:44Z

Over all, I think the approaches you've identified so far are the right ones to describe. I browsed some of the community solutions and most of them qualify as one of those approaches or a mix of two.

I found one extra approach that I would call "I love nesting", but I wouldn't recommend it to anyone 😁

def leap_year?(year) do
    if rem(year, 4) == 0 do
      if rem(year, 100) == 0 do
        if rem(year, 400) == 0 do
          true
        else
          false
        end
      else
        true
      end
    else
      false
    end
  end

Co-authored-by: Angelika Tyborska <[email protected]>

michalporeba · 2024-01-10T18:03:37Z

Thanks for working on this! I personally don't have the motivation to write any approaches myself, but if do, I promise to review all your PRs on time 😇.

Thanks. I think I'm the opposite. I like the approaches and writing about ways to solve problems with code. I don't know for how long I will have enough energy to do so, but I'll approach this problem a week at the time. My objective is to finish this by the end of the week with all the detailed approaches. I know there is no reverse exercise for the second week, so it will give us (me) some time to prepare approaches for the following week.

For the future, it's better to tell me explicitly that you want a draft PR reviewed when you open it. I usually wait until it's marked as "ready for review" because, well... that's the name of the feature 😂 and some people are shy about having their work in progress judged.

I have requested this on the forum, but I will make it more explicit in the PRs, too. Generally, here I'm also the opposite. "ready for review" for me is "ready for the final review and merge", while the "draft" is for early feedback that can lead to changes in direction. I'm not shy in that regard. I love early feedback and for my work in progress to be judged.

Many comments you provided today will be very helpful as I write more text in the approaches to reduce, hopefully, the number of corrections required.

Dzięki!

angelikatyborska · 2024-01-13T07:51:49Z

should it be multiple clause functions or multiple function clauses?

What's the context?

Here's a quick reference to how both phrases can be used correctly:

angelikatyborska · 2024-01-13T07:58:04Z

exercises/practice/leap/.approaches/operators/content.md

+At the core of this approach, three checks are returning three boolean values. 
+We can use [Boolean logic](https://en.wikipedia.org/wiki/Boolean_algebra) to combine the results.
+
+When using this approach, it is essential to consider short-circuiting of boolean operators. 


I'd say short-circuiting does not matter at all in this exercise. Short-circuiting is important to consider when we want to avoid or ensure some side effects, for example:

iex()> 40 / x ** (ArithmeticError) bad argument in arithmetic expression: 40 / 0 :erlang./(40, 0) iex:3: (file) iex()> x != 0 && 40 / x false

In this exercise, all inputs are valid, all the checks on either side of the boolean operator are pure functions with no side effects.

Here are the reasons why I think it is still relevant, even if all inputs are valid, all the checks are pure functions.

In the approach I'm showing different variations of the code. The option with the custom divides?/2 function is not necessarily guaranteed to be pure.

The approach at the end I'm showing an option to capture the results in variables. Perhaps I should make it more explicit, but in this case, all checks are performed because the short-circuiting applies only to checking the value of the variables.

In every exercise instructions, at the bottom, there instructions on how to use IO.inspect/1 for debugging your solutions. It is possible, that somebody less experienced will be surprised that inspect added to a custom function, or perhaps directly to one of the checks. Consider this code

rem(year, 4) == 0 and not IO.inspect(rem(year, 100) == 0) or rem(year, 400) == 0

When rem(year, 4) != 0 the IO.inspect/1 will not show anything because it will not be evaluated.

ok, I'm convinced 🙂

exercises/practice/leap/.approaches/introduction.md

exercises/practice/leap/.approaches/functions/content.md

exercises/practice/leap/.approaches/flow/content.md

angelikatyborska · 2024-01-13T08:22:51Z

What do you think about the level of detail? Too little? Too much?

I have no idea! What do other tracks do?

How about external links?

Are you asking if it's too many or not enough? Or are you asking about their quality?

Some of the formatting is difficult to see and evaluate unless the deep dive page is visible on Exercism. That's something I have learnt in other tracks. I think we should assume that despite doing our best to get it right, there might need to be a subsequent PR with a few corrections (e.g., headers, spacing around code examples, snippet formatting).

Agreed

Co-authored-by: Angelika Tyborska <[email protected]>

michalporeba · 2024-01-13T11:19:55Z

What do you think about the level of detail? Too little? Too much?

I have no idea! What do other tracks do?

The approach to approaches the tracks take are not very consistent. In that sense, the approaches are similar to concepts and exercise introductions. Some tracks are very detailed to the point of becoming a language tutorial. Other tracks are much more dry. Concepts can be lengthy and detailed, but they can only be more or less code examples.

Personally, I like the approaches, because they offer more than what can be seen in the community solutions. They offer an opportunity to explain language features in context and how they apply to a problem. You can help somebody expand their knowledge and understanding.

I hope they can help people like siraj-samsudeen, who wrote on the forum:

When I looked at the community solutions after submitting my solution, the top-rated solutions were greek-and-latin to me - i could NOT make out much of what they were doing. There was no explanation in the code.

What I'm trying to do when I decide what to include and what not to do is go back to this explanation of the idea by Jeremy.

But, as I said, each track is different. As you are leading the Elixir one, I wonder what you think about it. If you don't normally look at approaches, the question might be more what you think about what I'm proposing here. If it works, I'll try to stay consistent from now on.

How about external links?

Are you asking if it's too many or not enough? Or are you asking about their quality?

Yes, both. I'm trying to be informative but not to explain everything that can be linked. I am also trying to limit the links to exercism and hexdocs.

I have adjusted the text to make it clear that while there are many ways to use a case statement, the case on the tuple is the most idiomatic one.

michalporeba · 2024-01-13T11:59:48Z

@angelikatyborska, it's been a long thread. Thanks for all the feedback. To help finalise this PR I think we are down to four things to decide on / adjust.

Better slug for functions. My preferences are in this order: clauses, function-clauses, multiple-function-clauses. I favour shorter slugs to match the other two: operators and flow. But I'd be happy to move to longer for all of them. Perhaps: control-flow, function-clauses, boolean-operators.
using the non-negative word. I think this is more accurate, but I would like to change the number to divisor there to align with the mathematical terms of the module operation.
short-circuiting. I'd like to keep it. A longer justification is available above.
variations on case. I'd like to keep it with the adjusted wording to explain that case on a tuple is the most idiomatic approach.

I'd like to finish this before the end of the weekend if you are available. I'll be available from time to time until about 20:00 today (your time). Tomorrow I will be available much less, mostly in the evening.

angelikatyborska · 2024-01-13T12:12:23Z

As you are leading the Elixir one, I wonder what you think about it. If you don't normally look at approaches, the question might be more what you think about what I'm proposing here. If it works, I'll try to stay consistent from now on.

If you want to take the detailed approach, I'm perfectly fine with that. I will just make sure that all the information in there is correct and consistent with the other docs 🙂

To help finalise this PR I...

To answer shortly:

Better slug for functions

I'm fine with keeping it short as long it's accurate, "clauses" is short and accurate, so go for that!

using the non-negative word

It was just a nitpick, not a problem, can stay!

short-circuiting

also can stay

variations on case. I'd like to keep it with the adjusted wording to explain that case on a tuple is the most idiomatic approach.

alright

I'll try to double-check the links later today and I'll make sure that the PR gets merged until the end of the weekend 👍

angelikatyborska

I pushed two tiny commits with some reformatting and grammar fixes so that it's faster than leaving comments about them.

I have no thoughts about the links 😅 looks fine to me.

Let's merge and see how it looks on the website, maybe people will have some feedback.

I'm approving and you can press the merge button when you're ready 🚀

angelikatyborska · 2024-01-13T13:26:41Z

Hmm, or maybe not, let's see those CI failures 🙃

angelikatyborska · 2024-01-13T13:28:38Z

Right, the link checker is going to fail because there are links to the new content that's not online yet:


## Errors per input

### Errors in exercises/practice/leap/.approaches/clauses/content.md

* [404] [https://exercism.org/tracks/elixir/exercises/leap/approaches/operators](https://exercism.org/tracks/elixir/exercises/leap/approaches/operators) | Failed: Network error: Not Found

### Errors in exercises/practice/leap/.approaches/flow/content.md

* [404] [https://exercism.org/tracks/elixir/exercises/leap/approaches/clauses](https://exercism.org/tracks/elixir/exercises/leap/approaches/clauses) | Failed: Network error: Not Found

### Errors in exercises/practice/leap/.approaches/introduction.md

* [404] [https://exercism.org/tracks/elixir/exercises/leap/approaches/clauses](https://exercism.org/tracks/elixir/exercises/leap/approaches/clauses) | Failed: Network error: Not Found
* [404] [https://exercism.org/tracks/elixir/exercises/leap/approaches/cond](https://exercism.org/tracks/elixir/exercises/leap/approaches/cond) | Failed: Network error: Not Found
* [404] [https://exercism.org/tracks/elixir/exercises/leap/approaches/operators](https://exercism.org/tracks/elixir/exercises/leap/approaches/operators) | Failed: Network error: Not Found

We can ignore that and merge anyway

michalporeba · 2024-01-13T14:44:46Z

@angelikatyborska I cannot merge, you'll have to do the button pushing.

But let's do it and see how it looks, get some feedback.

angelikatyborska · 2024-01-13T15:59:36Z

Ah shit, sorry, I keep forgetting how it works with permissions 😬

angelikatyborska · 2024-01-13T16:04:44Z

It is live: https://exercism.org/tracks/elixir/exercises/leap/dig_deeper

thanks again! first ever Elixir approaches :)

angelikatyborska · 2024-01-13T16:13:21Z

exercises/practice/leap/.approaches/introduction.md

+[boolean-operators]: https://hexdocs.pm/elixir/operators.html#general-operators
+[operators-approach]: https://exercism.org/tracks/elixir/exercises/leap/approaches/operators
+[clause-approach]: https://exercism.org/tracks/elixir/exercises/leap/approaches/clauses
+[flow-approach]: https://exercism.org/tracks/elixir/exercises/leap/approaches/cond


This one broken link snuck in to main. Could you open a PR with a fix? If you do it, I can just merge, but if I do it, I will need to wait for another person with write rights to review because I can't approve my own PRs 😇

michalporeba added 2 commits January 6, 2024 20:25

setting up the structure for leap approaches

c4108ab

first draft of the introduction

630a266

michalporeba added 5 commits January 6, 2024 21:31

adding myself to the exercise contributors

d4084e0

fixing formatting

2b8ec47

snippets and headers as placeholders

616f6ec

improving the code with divides?

0a4fddc

more explicit parameters in divides?

5038fcb

Tuxified reviewed Jan 8, 2024

View reviewed changes

exercises/practice/leap/.approaches/case/snippet.txt Outdated Show resolved Hide resolved

Tuxified reviewed Jan 8, 2024

View reviewed changes

exercises/practice/leap/.approaches/cond/snippet.txt Outdated Show resolved Hide resolved

Tuxified approved these changes Jan 8, 2024

View reviewed changes

Include approach snippets in formatting

0e16a73

angelikatyborska reviewed Jan 10, 2024

View reviewed changes

michalporeba and others added 6 commits January 10, 2024 17:51

Update exercises/practice/leap/.approaches/introduction.md

0d19aa2

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/introduction.md

54a79e1

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/introduction.md

3c81c2c

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/introduction.md

09d55ca

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/introduction.md

085dfa3

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/introduction.md

6e85e4f

Co-authored-by: Angelika Tyborska <[email protected]>

michalporeba added 3 commits January 10, 2024 21:42

reformatting

5eb7841

boolean operators approach

510c33c

rem by default, divides? as an option

fc4a44c

angelikatyborska reviewed Jan 13, 2024

View reviewed changes

michalporeba and others added 6 commits January 13, 2024 10:33

Update exercises/practice/leap/.approaches/introduction.md

deb494f

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/flow/content.md

ff5bc09

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/flow/content.md

8db3e1e

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/functions/content.md

0754b52

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/introduction.md

33e7fc6

Co-authored-by: Angelika Tyborska <[email protected]>

Update exercises/practice/leap/.approaches/introduction.md

a930e7f

Co-authored-by: Angelika Tyborska <[email protected]>

Case on the tuple is the most idiomatic

06d05d2

I have adjusted the text to make it clear that while there are many ways to use a case statement, the case on the tuple is the most idiomatic one.

typo

19a3bb6

michalporeba and others added 4 commits January 13, 2024 12:27

clauses not functions

6e527c7

the number is a divisor

16f7034

Reformat code in code blocks

5c8503c

Run through a grammar checker

a2172ae

angelikatyborska approved these changes Jan 13, 2024

View reviewed changes

Format approach snippets

5a8d670

angelikatyborska merged commit 841d716 into exercism:main Jan 13, 2024
8 of 10 checks passed

angelikatyborska reviewed Jan 13, 2024

View reviewed changes

This was referenced Jan 13, 2024

Leap 48in24 approaches fixes #1409

Closed

Improving on the first draft of the Leap approaches #1410

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leap 48in24 approaches #1401

Leap 48in24 approaches #1401

michalporeba commented Jan 6, 2024

github-actions bot commented Jan 6, 2024

Tuxified commented Jan 8, 2024 •

edited

Loading

angelikatyborska commented Jan 8, 2024

angelikatyborska commented Jan 8, 2024

michalporeba commented Jan 8, 2024

michalporeba commented Jan 9, 2024

angelikatyborska commented Jan 9, 2024

angelikatyborska left a comment

angelikatyborska Jan 10, 2024

michalporeba Jan 10, 2024

angelikatyborska Jan 13, 2024

angelikatyborska commented Jan 10, 2024

michalporeba commented Jan 10, 2024

angelikatyborska commented Jan 13, 2024

angelikatyborska Jan 13, 2024

michalporeba Jan 13, 2024

angelikatyborska Jan 13, 2024

angelikatyborska commented Jan 13, 2024

michalporeba commented Jan 13, 2024

michalporeba commented Jan 13, 2024 •

edited

Loading

angelikatyborska commented Jan 13, 2024

angelikatyborska left a comment

angelikatyborska commented Jan 13, 2024

angelikatyborska commented Jan 13, 2024

michalporeba commented Jan 13, 2024

angelikatyborska commented Jan 13, 2024

angelikatyborska commented Jan 13, 2024 •

edited

Loading

angelikatyborska Jan 13, 2024

		The two functions differ in how they work with negative numbers, but since, in this exercise,
		all the numbers are non-negative, both could work, depending on the approach.

Leap 48in24 approaches #1401

Leap 48in24 approaches #1401

Conversation

michalporeba commented Jan 6, 2024

github-actions bot commented Jan 6, 2024

Tuxified commented Jan 8, 2024 • edited Loading

angelikatyborska commented Jan 8, 2024

angelikatyborska commented Jan 8, 2024

michalporeba commented Jan 8, 2024

michalporeba commented Jan 9, 2024

angelikatyborska commented Jan 9, 2024

angelikatyborska left a comment

Choose a reason for hiding this comment

angelikatyborska Jan 10, 2024

Choose a reason for hiding this comment

michalporeba Jan 10, 2024

Choose a reason for hiding this comment

angelikatyborska Jan 13, 2024

Choose a reason for hiding this comment

angelikatyborska commented Jan 10, 2024

michalporeba commented Jan 10, 2024

angelikatyborska commented Jan 13, 2024

angelikatyborska Jan 13, 2024

Choose a reason for hiding this comment

michalporeba Jan 13, 2024

Choose a reason for hiding this comment

angelikatyborska Jan 13, 2024

Choose a reason for hiding this comment

angelikatyborska commented Jan 13, 2024

michalporeba commented Jan 13, 2024

michalporeba commented Jan 13, 2024 • edited Loading

angelikatyborska commented Jan 13, 2024

angelikatyborska left a comment

Choose a reason for hiding this comment

angelikatyborska commented Jan 13, 2024

angelikatyborska commented Jan 13, 2024

michalporeba commented Jan 13, 2024

angelikatyborska commented Jan 13, 2024

angelikatyborska commented Jan 13, 2024 • edited Loading

angelikatyborska Jan 13, 2024

Choose a reason for hiding this comment

Tuxified commented Jan 8, 2024 •

edited

Loading

michalporeba commented Jan 13, 2024 •

edited

Loading

angelikatyborska commented Jan 13, 2024 •

edited

Loading