Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

date-parser improvements #941

Closed
Cohen-Carlisle opened this issue Oct 1, 2021 · 6 comments · Fixed by #1148
Closed

date-parser improvements #941

Cohen-Carlisle opened this issue Oct 1, 2021 · 6 comments · Fixed by #1148
Assignees

Comments

@Cohen-Carlisle
Copy link
Member

Cohen-Carlisle commented Oct 1, 2021

As mentioned on slack:
Doing the date-parser exercise has been weird for me. Whenever I do regex, I try to use the std lib as much as possible and keep my regex as simple as possible. I find this makes much more maintainable code. But this exercise was exclusively writing regexes to pass certain inputs, and almost all as Strings being Regex.compile!ed instead of actual regexes.

I think the general way I would have approached this would have been to take the input formats:

  1. "01/01/1970"
  2. "January 1, 1970"
  3. "Thursday, January 1, 1970"
    and try to parse a Date from each. So for example for 1, I would probably avoid Regex altogether and do something like:
"01/2/1970"
|> String.split("/")
|> Enum.map(&String.pad_leading(&1, 2, "0"))
|> Enum.reverse()
|> Enum.join("-")
|> Date.from_iso8601()

If that came back error, I would try to parse 2 and 3 with regex:

regex = ~r/^(?:[a-z]+, )?([a-z]+) (\d+), (\d+)$/i
Regex.run(regex, "January 1, 1970")
# => ["January 1, 1970", "January", "1", "1970"]
Regex.run(regex, "Someday, January 1, 1970")
# => ["Someday, January 1, 1970", "January", "1", "1970"]

Then I can take the last 3 elements and feed them into Date.new after integerizing and reordering them.

But... I don't know if the above would accomplish the learning objectives of the exercise.
As a smaller fix, I noticed my test output wasn't particularly helpful in my workflow.
My workflow was basically implement a function, run the tests, then see the next failing test to inform me on how to proceed. But since the functions are being piped into Regex.compile!, I would just get function clause errors. Very late I caught on that I could get more helpful errors if I just put "" as the body of the next function.

I also took a look at the exemplar code and liked the grouping it achieved, but unfortunately that grouping isn't natural the way the stub lib file comes.

@neenjaw
Copy link
Contributor

neenjaw commented Oct 1, 2021

So you are correct, there are more ways to tackle this problem with more use of the standard lib and less use of regex, but the concept exercise is contrived to teach regex patterns. So while I agree that if doing this for myself I might use a different route, how can we focus on making this a better regex learning exercise?

Whether we adapt and improve this exercise or deprecate and create a new one is fine, but there does need to be a progression of simple patterns to complex patterns and I think constraining it to simple matching makes it easier than opening up the flood gates to Regex.scan and Regex.run.

@Cohen-Carlisle
Copy link
Member Author

I think one way would be to let them use Regex.match? directly instead of passing the regex returned by the match_* functions into it. E.g, instead of writing

def match_numeric_date(), do: ~r/^#{capture_numeric_date()}$/

we could have them write:

def matches_numeric_date?(string), do: Regex.match?(~r/^#{capture_numeric_date()}$/, string)

More generally, I think what I'm really feeling is that if I were to teach people regex, I'd want them to use std lib and write just a few, simple regexes, but here I'm not using std lib and I end up making a lot of different regexes with interpolation.

@octowombat
Copy link

I also found this exercise weird too as it's possible to get all the tests to pass even if the first function day/0 has an implementation that allows an impossible day value to be parsed, such as say, 32. The implementation is as simple as

  @spec day() :: String.t()
  def day do
    "\\d{1,2}"
  end

@angelikatyborska
Copy link
Contributor

Adding @ErikSchierboom's feedback:

My least favorite exercise was the date-parser exercise, which teaches regular expressions. I found the implementation quite repetitive. Many of the methods that needed to be implemented did not really teach me anything new, but just required slight tweaks to what I had done before. There were also a lot of unit tests, which made the mix test output somewhat hard to work with

@angelikatyborska angelikatyborska self-assigned this Jun 12, 2022
@angelikatyborska
Copy link
Contributor

I have started working on replacing date-parser with log-parser which will be a fork of https://github.com/exercism/go/tree/main/exercises/concept/parsing-log-files

@ErikSchierboom
Copy link
Member

I did the log-parser exercise and liked it a lot better! Great work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants