refactor: update trivial regex capture group find and replace challenge #37768

PartyLich · 2019-11-16T01:34:59Z

Implements the fixes provided by @alanpaulprice in pull #36925 with
the changes requested by maintainers. Previous challenge could be
completed without any capture groups. New challenge rectifies this
behavior.

resolves #17645

I have read freeCodeCamp's contribution guidelines.
My pull request has a descriptive title (not a vague title like Update index.md)
My pull request targets the master branch of freeCodeCamp.
None of my changes are plagiarized from another source without proper attribution.
Note: as the commit text says, this is the pull referenced above with the requested corrections. Not my work, basically.
All the files I changed are in the same world language (for example: only English changes, or only Chinese changes, etc.)
My changes do not use shortened URLs or affiliate links.

Closes #17645

gitpod-io · 2019-11-16T01:35:03Z

RandellDawson

@PartyLich In addition to the change I have suggested, there should be an additional test which validates at least three capture groups were used. Otherwise, a simple replace would solve the challenge without even using regex.

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md

RandellDawson

See my suggested changes.

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md

RandellDawson

@PartyLich Let's change the huhText which means nothing to str.

Also, you need to figure out a way to not allow a solution like the following which does not use replacement patterns:

let str = "one two three";
let fixRegex = /(\w+) (\w+) (\w+)/; // Change this line
let replaceText = "three two one"; // Change this line
let result = str.replace(fixRegex, replaceText);

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md

PartyLich · 2019-11-19T03:09:16Z

We could use a regex to ensure there is some instance of $n with n in the 1-99 range, but I'm not certain at the moment what new holes that approach would have.

- text: <code>replaceText</code> should use parenthesized submatch string(s).
    testString: assert(replaceText.match(/(\$\d{1,2})+(?:[\D]|\b)/g));

Do we care that they use >1 submatch in the final replacement?

It would make sense to me to use all 3 of the mandated capture groups in the replacement as well, but idk if we want to enforce that.

PartyLich · 2019-11-28T01:30:34Z

new test added.
passing:

let str = "one two three";
let fixRegex = /(\w+) (\w+) (\w+)/; // Change this line
let replaceText = "$3 $2 $1"; // Change this line
let result = str.replace(fixRegex, replaceText);

let replaceText = "three $2 $1"; // Change this line

let replaceText = "three two $1"; // Change this line

failing

let str = "one two three";
let fixRegex = /(\w+) (\w+) (\w+)/; // Change this line
let replaceText = "three two one"; // Change this line
let result = str.replace(fixRegex, replaceText);

"Parenthesized submatch string" is interesting wording...but it matches MDN should a student happen to use that source for research/help. I'm obviously open to suggested replacements.

RandellDawson · 2019-11-28T02:36:24Z

I do not like the fact that the following pass:

let replaceText = "three $2 $1"; // Change this line

let replaceText = "three two $1"; // Change this line

The user should only be using spaces and $n for the replacement and not hard-coded values.

PartyLich · 2019-11-28T17:03:56Z

I do not like the fact that the following pass:
let replaceText = "three $2 $1"; // Change this line
let replaceText = "three two $1"; // Change this line
The user should only be using spaces and $n for the replacement and not hard-coded values.

That's why my previous comment included the question "Do we care that they use >1 submatch in the final replacement?". Whether they use 1 or 3 or n, they're still demonstrating that they are familiar with the concept and can utilize it to update a string.

RandellDawson · 2019-11-28T19:47:08Z

Let's make sure they use all 3.

RandellDawson · 2019-12-02T01:30:46Z

OK, I think this one looks good now.

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md

Manish-Giri · 2019-12-02T22:12:14Z

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md

+    testString: assert(code.match(/result\s*=\s*str\.replace\(.*?\)/));
+  - text: <code>fixRegex</code> should use at least three capture groups.
+    testString: assert((new RegExp(fixRegex.source + '|')).exec('').length - 1 >= 3);
+  - text: <code>replaceText</code> should use parenthesized submatch string(s).


replaceText should use parenthesized submatch string(s).

If I'm new to regex, going solely by the challenge description, I have no idea what "parenthesized submatch string(s)" means. Probably should be rephrased.

Any suggestions on what to rephrase it to? I didn't come across or previously know an alternate term. If I were a new(er) student and making use of the "Read-Search-Ask" slogan that was so prominent on FCC, a google search of the phrase would produce accurate results (for multiple programming languages). If I were me and had the habit of looking up js docs on MDN, the String.prototype.replace() page has this exact wording.

I have no strong feelings about changing it to something else, I just have no idea what would be better. I could use some help there.

An/Or I suppose we could add the preferred terminology to the description. At present it says something to the effect of "use dollar signs", but offers no terminology to concisely describe that.

Since you have now edited the instructions to add a little detail about the $ syntax, perhaps this could be (re)used to simplify the terminology -

text: replaceText should use parenthesized submatch string(s) (ie. the nth parenthesized submatch string, $n, corresponds to the nth capture group).

The MDN docs on .replace() uses this terminology.

Manish-Giri

In addition to the suggested changes, the challenge instruction should also include what to do with the fixRegex and replaceText variables in the editor. Currently, the instruction just states -

Write a regex using three capture groups that will replace the string "one two three" with the string "three two one" and assign the result to the replaceText variable.

You shouldn't be required to look at the tests to figure out what the variables are for -

fixRegex should use at least three capture groups.
replaceText should use parenthesized submatch string(s).

We could use something on the lines of what we had in the previous version -

Write a regex so that it will search for the string "good". Then update the replaceText variable to replace "good" with "okey-dokey"

Previous challenge could be completed without any capture groups. New challenge requires 3 capture groups and 3 uses of the submatches in the replacement. resolves #17645

@RandellDawson

Use solution from SO (https://stackoverflow.com/questions/16046620/regex-to-count-the-number-of-capturing-groups-in-a-regex) as suggested by @RandellDawson to count the number of capturing groups, regardless of whether or not the regex in question solves the broader problem. Co-Authored-By: Randell Dawson <5313213+RandellDawson@users.noreply.github.com>

Co-Authored-By: Randell Dawson <5313213+RandellDawson@users.noreply.github.com>

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md

Manish-Giri

Minor grammar fix.

Manish-Giri · 2019-12-05T04:52:22Z

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md

+    testString: assert(code.match(/result\s*=\s*str\.replace\(.*?\)/));
+  - text: <code>fixRegex</code> should use at least three capture groups.
+    testString: assert((new RegExp(fixRegex.source + '|')).exec('').length - 1 >= 3);
+  - text: <code>replaceText</code> should use parenthesized submatch string(s).


Since you have now edited the instructions to add a little detail about the $ syntax, perhaps this could be (re)used to simplify the terminology -

text: replaceText should use parenthesized submatch string(s) (ie. the nth parenthesized submatch string, $n, corresponds to the nth capture group).

The MDN docs on .replace() uses this terminology.

Co-Authored-By: Manish Giri <manish.giri.me@gmail.com>

Manish-Giri

LGTM. Thank You for your contribution.

@RandellDawson

…ge (freeCodeCamp#37768) * refactor: replace trivial regex capture group challenge Previous challenge could be completed without any capture groups. New challenge requires 3 capture groups and 3 uses of the submatches in the replacement. resolves freeCodeCamp#17645 * test: validate at least 3 capture groups were used Use solution from SO (https://stackoverflow.com/questions/16046620/regex-to-count-the-number-of-capturing-groups-in-a-regex) as suggested by @RandellDawson to count the number of capturing groups, regardless of whether or not the regex in question solves the broader problem. Co-Authored-By: Randell Dawson <5313213+RandellDawson@users.noreply.github.com> * refactor: more meaningful variable name Co-Authored-By: Randell Dawson <5313213+RandellDawson@users.noreply.github.com> * test: add parenthesized substring test * test: ensure 3 uses of substring replacement * refactor: reword instructions * fix: minor grammar fix Co-Authored-By: Manish Giri <manish.giri.me@gmail.com> * refactor: elaborate wording in test

RandellDawson suggested changes Nov 16, 2019

View reviewed changes

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md Outdated Show resolved Hide resolved

RandellDawson mentioned this pull request Nov 16, 2019

Problem with Guide solution and challenge solution for Reuse Patterns Using Capture Groups challenge #35765

Closed

PartyLich requested a review from RandellDawson November 16, 2019 19:39

RandellDawson suggested changes Nov 16, 2019

View reviewed changes

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md Outdated Show resolved Hide resolved

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md Outdated Show resolved Hide resolved

PartyLich requested a review from RandellDawson November 19, 2019 01:20

RandellDawson suggested changes Nov 19, 2019

View reviewed changes

RandellDawson added the status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP label Nov 19, 2019

camperbot added language: English scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. labels Nov 19, 2019

PartyLich requested a review from RandellDawson November 28, 2019 01:36

RandellDawson approved these changes Dec 2, 2019

View reviewed changes

RandellDawson removed the status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP label Dec 2, 2019

Manish-Giri reviewed Dec 2, 2019

View reviewed changes

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md Outdated Show resolved Hide resolved

Manish-Giri reviewed Dec 2, 2019

View reviewed changes

Manish-Giri suggested changes Dec 2, 2019

View reviewed changes

PartyLich and others added 6 commits December 3, 2019 20:45

refactor: replace trivial regex capture group challenge

13e28e5

Previous challenge could be completed without any capture groups. New challenge requires 3 capture groups and 3 uses of the submatches in the replacement. resolves #17645

refactor: more meaningful variable name

39fcaac

Co-Authored-By: Randell Dawson <5313213+RandellDawson@users.noreply.github.com>

test: add parenthesized substring test

c67fbb8

test: ensure 3 uses of substring replacement

5f0a6ef

refactor: reword instructions

a122e00

Manish-Giri reviewed Dec 5, 2019

View reviewed changes

...-and-data-structures/regular-expressions/use-capture-groups-to-search-and-replace.english.md Outdated Show resolved Hide resolved

Manish-Giri suggested changes Dec 5, 2019

View reviewed changes

fix: minor grammar fix

f65e5d9

Co-Authored-By: Manish Giri <manish.giri.me@gmail.com>

Manish-Giri added the status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP label Dec 8, 2019

refactor: elaborate wording in test

3e9e46c

PartyLich requested a review from Manish-Giri December 14, 2019 07:16

Manish-Giri removed the status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP label Dec 14, 2019

Manish-Giri approved these changes Dec 14, 2019

View reviewed changes

Manish-Giri merged commit 616d96f into freeCodeCamp:master Dec 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: update trivial regex capture group find and replace challenge #37768

refactor: update trivial regex capture group find and replace challenge #37768

PartyLich commented Nov 16, 2019

gitpod-io bot commented Nov 16, 2019 •

edited

RandellDawson left a comment

RandellDawson left a comment

RandellDawson left a comment

PartyLich commented Nov 19, 2019 •

edited

PartyLich commented Nov 28, 2019 •

edited by RandellDawson

RandellDawson commented Nov 28, 2019

PartyLich commented Nov 28, 2019

RandellDawson commented Nov 28, 2019

RandellDawson commented Dec 2, 2019

Manish-Giri Dec 2, 2019

PartyLich Dec 4, 2019

PartyLich Dec 4, 2019

Manish-Giri Dec 5, 2019

Manish-Giri left a comment

Manish-Giri left a comment

Manish-Giri Dec 5, 2019

Manish-Giri left a comment

refactor: update trivial regex capture group find and replace challenge #37768

refactor: update trivial regex capture group find and replace challenge #37768

Conversation

PartyLich commented Nov 16, 2019

gitpod-io bot commented Nov 16, 2019 • edited

RandellDawson left a comment

Choose a reason for hiding this comment

RandellDawson left a comment

Choose a reason for hiding this comment

RandellDawson left a comment

Choose a reason for hiding this comment

PartyLich commented Nov 19, 2019 • edited

PartyLich commented Nov 28, 2019 • edited by RandellDawson

RandellDawson commented Nov 28, 2019

PartyLich commented Nov 28, 2019

RandellDawson commented Nov 28, 2019

RandellDawson commented Dec 2, 2019

Manish-Giri Dec 2, 2019

Choose a reason for hiding this comment

PartyLich Dec 4, 2019

Choose a reason for hiding this comment

PartyLich Dec 4, 2019

Choose a reason for hiding this comment

Manish-Giri Dec 5, 2019

Choose a reason for hiding this comment

Manish-Giri left a comment

Choose a reason for hiding this comment

Manish-Giri left a comment

Choose a reason for hiding this comment

Manish-Giri Dec 5, 2019

Choose a reason for hiding this comment

Manish-Giri left a comment

Choose a reason for hiding this comment

gitpod-io bot commented Nov 16, 2019 •

edited

PartyLich commented Nov 19, 2019 •

edited

PartyLich commented Nov 28, 2019 •

edited by RandellDawson