Skip to content

Conversation

@makingthematrix
Copy link

No description provided.

5. And the substring should finish with the closing parentheses, escaped again, so `\)`.
That's it. Our whole regex looks like this:
```scala
val mulPattern: Regex = """mul\((\d+)\,(\d+)\)""".r
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

, doesn't need to be quoted:

Suggested change
val mulPattern: Regex = """mul\((\d+)\,(\d+)\)""".r
val mulPattern: Regex = """mul\((\d+),(\d+)\)""".r

Also, have you considered using java.util.regex.Pattern.quote or the \Q...\E escapes? They might not do much good for mul, but for do() and don't() that would probably make it easier to read.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, nice. I focused on other parts and somehow I just though , needs to be escaped and didn't check it.

I thought that for this task it'd be more consistent if I didn't use any additional method calls. Since I already discussed escaping parentheses in "mul", then I just did the same for "do" and "don't".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made one more commit, removing the part about escaping commas.

Copy link
Collaborator

@KacperFKorban KacperFKorban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add yourself as the author of the solution, as with the first one? https://scalacenter.github.io/scala-advent-of-code/2024/puzzles/day01

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added my solution in a separate commit. It's already merged.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I think #590 is what Kacper meant)

@SethTisue SethTisue merged commit 7d2b501 into scalacenter:website Dec 4, 2024
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 this pull request may close these issues.

4 participants