Added Exact Value/s Condition for String Length & Tests required with README.md Update #2019

bevatsal1122 · 2022-08-06T18:48:25Z

feat(isLength): Added Conditions for checking if String's length is equal to exact if exact provided

If the string length falls in the range and if exact number/array/object is provided, then exact conditions are checked. If not provided, then true is returned is len>=min & len<=max and false is returned is not in the range [min, max]. README.md File is also updated for the same.

Checklist

README updated (where applicable)
PR contains only changes related; no stray files, etc.
Tests written (where applicable)

codecov · 2022-08-06T18:54:53Z

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (531dc7f) 100.00% compared to head (3ef2588) 100.00%.

❗ Current head 3ef2588 differs from pull request most recent head 22bb2f0. Consider uploading reports for the commit 22bb2f0 to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #2019   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          104       104           
  Lines         2308      2311    +3     
  Branches       578       579    +1     
=========================================
+ Hits          2308      2311    +3

Impacted Files	Coverage Δ
src/lib/isLength.js	`100.00% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

bevatsal1122 · 2022-08-06T21:07:19Z

Hello, I have added the required feature. If the string length falls in the range and if exact number/array/object is mentioned, then exact conditions are checked. If not provided, then true is returned is len>=min & len<=max and false is returned is not in range [min, max]. All the checks have also been passed :)
PR#2019 - #2019

WikiRik · 2022-08-08T10:05:08Z

src/lib/isLength.js

  } else { // backwards compatibility: isLength(str, min [, max])
    min = arguments[1] || 0;
    max = arguments[2];
+    exact = arguments[3];


I don't think we need backwards compatibility for a new feature

That also removes the need for some of the tests btw

Ohh I thought if at all a User wants to check if it's exact to a bunch of values rather than a single value, so I configured the exact value to be an array or an object or a number. Should I remove the backwards compatibility feature ??

It's just this line that you can remove (and the tests associated to this line). This else statement is only here for the people that use the old syntax for isLength. And since this is a new feature there is no need to implement it in both the old and new syntax.

That you configured the exact value to be an array or object or number is fine.

Okkay!! I made the required changes & also updated the Tests for the same... Hence, now this new Feature exact is implemented only in the new Syntax of isLength.

WikiRik · 2022-08-08T10:08:32Z

src/lib/isLength.js

@@ -5,15 +5,42 @@ export default function isLength(str, options) {
  assertString(str);
  let min;
  let max;
+  let exact;
+  let result;
+  let isPerfect = false;


Maybe refactor this to isValid?

Yess!! Renamed isPerfect to isValid

WikiRik

Thanks for the updates! I have some additional comments

And I'll tag @rubiin here as well for the more official review and approval for having exact be either a number, a array or an object of numbers since I'm not a maintainer of this library

WikiRik · 2022-08-13T19:27:39Z

README.md

@@ -140,7 +140,7 @@ Validator                               | Description
 **isJSON(str [, options])**             | check if the string is valid JSON (note: uses JSON.parse).<br/><br/>`options` is an object which defaults to `{ allow_primitives: false }`. If `allow_primitives` is true, the primitives 'true', 'false' and 'null' are accepted as valid JSON values.
 **isJWT(str)**                         | check if the string is valid JWT token.
 **isLatLong(str [, options])**                      | check if the string is a valid latitude-longitude coordinate in the format `lat,long` or `lat, long`.<br/><br/>`options` is an object that defaults to `{ checkDMS: false }`. Pass `checkDMS` as `true` to validate DMS(degrees, minutes, and seconds) latitude-longitude format.
-**isLength(str [, options])**              | check if the string's length falls in a range.<br/><br/>`options` is an object which defaults to `{min:0, max: undefined}`. Note: this function takes into account surrogate pairs.
+**isLength(str [, options])**              | check if the string's length falls in a range and equal to exactValue if provided.<br/><br/>`options` is an object which defaults to `{min:0, max: undefined, exact: undefined}`. Note: this function takes into account surrogate pairs.


Maybe add here that exact can be a single number, an array or an object of several numbers?

Done!! Updated the README.md.

WikiRik · 2022-08-13T19:28:24Z

test/validators.js

@@ -4629,6 +4629,12 @@ describe('Validators', () => {
      valid: [''],
      invalid: ['a', 'ab'],
    });
+    test({


I think we can remove this new test since it does not test the new functionality

Yess!! Removed that test.

WikiRik · 2022-08-13T19:29:53Z

test/validators.js

+      valid: ['helloguy', 'shopping', 'validator', 'length'],
+      invalid: ['abcde', 'abcdefg', 'abcdefghij'],
+    });
+    test({


We can remove this test since it tests the same as the test of { exact: 2 }.

Or adjust this test to test the combination of a min and exact values.

Actually, the later one with exact: { first: 6, second: 8, third: 9 } checks for the condition involving the exact being an object. Hence, I have adjusted the previous one with {exact: 2} to test the combination of a min and exact values.

I was wrong about this test, the difference is that this is using a string of '9' instead of the number 9. That makes this a different test to { exact: 2 }. So the combination of min and exact should be an additional test

So I have included the combination of a min and exact values in a test with args: [{ min: 1, exact: 2 }].

WikiRik · 2022-08-13T19:31:41Z

test/validators.js

+    });
+    test({
+      validator: 'isLength',
+      args: [{ min: 5, max: 10, exact: { first: 6, second: 8, third: 9 } }],


What would happen if one of the exact numbers is lower than min or higher than max? Should we add tests for that behaviour as well?

Since the exact condition only runs when the Length of the String falls inside [min, max] range, if at all exact integer or one of the exact numbers is out of range [min, max], the equality check for that case will return false and isValid won't be set to true.
Also, I have included the test condition for that behavior in the already present test with args: [{ min: 5, max: 10, exact: [2, 6, 8, 9] }].

src/lib/isLength.js

WikiRik · 2022-08-13T19:37:02Z

src/lib/isLength.js

+        isValid = true;
+      }
+    }
+  } else {


This else statement is not needed since you already define isValid as false at the beginning.

Yess!! Removed the else statement.

bevatsal1122 · 2022-11-14T16:13:23Z

@pixelbucket-dev I made all the necessary changes and also synced the branch with the latest commits.

pixelbucket-dev

LGTM, just a few comments :).

src/lib/isLength.js

pixelbucket-dev · 2022-11-14T20:15:31Z

I was just thinking: A future improvement could be to improve the options API to have consistent fields. This is because, with discreteLengths, we have a new field, and the API might become less intuitive to use now.

Something like:

{
	range: {
		min: 0,
		max: undefined
	}, // optional
	discreteLengths: undefined // optional
}

With the above proposal, it also becomes clear, that there is another permutation of the rules set:

range defined AND discreteLengths undefined ⇒ implemented by this PR
range undefined AND discreteLengths undefined ⇒ implemented by this PR
range defined AND discreteLengths defined ⇒ implemented by this PR
range undefined AND discreteLengths defined ⇒ NOT implemented by this PR

So basically, we would have to discuss if this API should allow checking against discrete lengths w/o using the range rule/option. Currently, to check discreteLengths, min and max, i.e. range, are also considered.

Just some food for thoughts 😄.

Co-authored-by: Falk Schieber <12937991+pixelbucket-dev@users.noreply.github.com>

bevatsal1122 · 2022-11-18T18:41:12Z

I was just thinking: A future improvement could be to improve the options API to have consistent fields. This is because, with discreteLengths, we have a new field, and the API might become less intuitive to use now.

Something like:
{
	range: {
		min: 0,
		max: undefined
	}, // optional
	discreteLengths: undefined // optional
}
With the above proposal, it also becomes clear, that there is another permutation of the rules set:

range defined AND discreteLengths undefined ⇒ implemented by this PR

range undefined AND discreteLengths undefined ⇒ implemented by this PR

range defined AND discreteLengths defined ⇒ implemented by this PR

range undefined AND discreteLengths defined ⇒ NOT implemented by this PR

So basically, we would have to discuss if this API should allow checking against discrete lengths w/o using the range rule/option. Currently, to check discreteLengths, min and max, i.e. range, are also considered.

Just some food for thoughts 😄.

Yeahh that's a really good idea. Making it segregated into key-value pairs would be easier to use and understand.
@WikiRik @rubiin What do you think ??

pixelbucket-dev · 2022-11-21T22:25:02Z

I was just thinking: A future improvement could be to improve the options API to have consistent fields. This is because, with discreteLengths, we have a new field, and the API might become less intuitive to use now.
Something like:
{
	range: {
		min: 0,
		max: undefined
	}, // optional
	discreteLengths: undefined // optional
}
With the above proposal, it also becomes clear, that there is another permutation of the rules set:

range defined AND discreteLengths undefined ⇒ implemented by this PR

range undefined AND discreteLengths undefined ⇒ implemented by this PR

range defined AND discreteLengths defined ⇒ implemented by this PR

range undefined AND discreteLengths defined ⇒ NOT implemented by this PR

So basically, we would have to discuss if this API should allow checking against discrete lengths w/o using the range rule/option. Currently, to check discreteLengths, min and max, i.e. range, are also considered.
Just some food for thoughts 😄.
Yeahh that's a really good idea. Making it segregated into key-value pairs would be easier to use and understand. @WikiRik @rubiin What do you think ??

It would be a breaking change! So I'd suggest keep the existing API (min and max), like with the other PRs here, and add the new fields range and discreteLengths. min and max should probably be prioritised if defined, for now. You can document the new syntax already in this PR and deprecate min and max. With the next major version, min and max will be removed.

WikiRik · 2022-12-02T17:55:19Z

@rubiin any reason why you approved this while the GitHub Actions have not passed?

pixelbucket-dev · 2023-01-03T19:48:54Z

@bevatsal1122 could you please fix the formatting issues that break the CI? View the “Files Changed” tab so see details about the linting issues.

N.B. we have to discuss better linter integration. Errors like this should never arrive in the PR, but rather fixed at commit time or push time by running commit hooks.

bevatsal1122 added 2 commits August 7, 2022 00:05

Added Exact Condition for String Length & Updated README.md

ae711d6

Fixed Small Issue

f93cf70

bevatsal1122 added 17 commits August 7, 2022 00:36

Updated test/validators.js

cdd2192

Fixed in test/validators.js

27a40c8

Updated test/validators.js for isLength()

cf729d1

Updated test/validators.js

be706c4

Updated test/validators.js

db5b0c7

Updated test/validators.js

2e48053

Updated test/validators.js

c194526

Updated test/validators.js

21ec415

Fixed Small Issue

59abfc7

Fixed Small Issue

e1bb810

Fixed Small Issue

7c2c0cf

Fixed Small Issue

3ee58c0

Fixed Small Issue

d3e6ca5

Fixed Small Issue

b850b84

Updated test/validators.js

acfe071

Updated test/validators.js

cdb06a5

Fixed Small Issue

70775b4

bevatsal1122 mentioned this pull request Aug 6, 2022

add more options in isLength - feature request #1937

Open

WikiRik reviewed Aug 8, 2022

View reviewed changes

Renamed isPerfect to isValid

a71b7ff

bevatsal1122 requested a review from WikiRik August 11, 2022 04:13

bevatsal1122 added 4 commits August 14, 2022 00:19

Removed Backwards Compatibility for exact Feature

dc42e7f

Fixed Minor Bug

fb0def3

Fixed Small Issue

ec1aa91

Fixed Small Issue

73dc505

WikiRik reviewed Aug 13, 2022

View reviewed changes

bevatsal1122 added 4 commits November 12, 2022 22:50

Fixed minor bug

d09738c

Minor test bug fixed

11aa19f

Fixed minor bug in test/validator.js

e63e7a5

Merge branch 'validatorjs:master' into MergeProposal

3ef2588

bevatsal1122 requested review from pixelbucket-dev and removed request for profnandaa November 14, 2022 16:13

pixelbucket-dev reviewed Nov 14, 2022

View reviewed changes

src/lib/isLength.js Outdated Show resolved Hide resolved

src/lib/isLength.js Outdated Show resolved Hide resolved

Update src/lib/isLength.js

3b5cbd9

Co-authored-by: Falk Schieber <12937991+pixelbucket-dev@users.noreply.github.com>

bevatsal1122 added 2 commits November 19, 2022 00:13

Added blank lines between statements

2897ef9

Updated isLength.js

22bb2f0

rubiin requested review from profnandaa, pixelbucket-dev, rubiin, WikiRik and tux-tn and removed request for pixelbucket-dev and WikiRik November 19, 2022 08:09

rubiin approved these changes Dec 2, 2022

View reviewed changes

profnandaa added 🧹 needs-update For PRs that need to be updated before landing and removed ready-to-land For PRs that are reviewed and ready to be landed ✅ LGTM labels Jan 22, 2023

bevatsal1122 closed this Jul 9, 2023

bevatsal1122 reopened this Jul 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Exact Value/s Condition for String Length & Tests required with README.md Update #2019

Added Exact Value/s Condition for String Length & Tests required with README.md Update #2019

bevatsal1122 commented Aug 6, 2022 •

edited

Loading

codecov bot commented Aug 6, 2022 •

edited

Loading

bevatsal1122 commented Aug 6, 2022

WikiRik Aug 8, 2022

WikiRik Aug 8, 2022

bevatsal1122 Aug 8, 2022

WikiRik Aug 12, 2022

bevatsal1122 Aug 13, 2022

WikiRik Aug 8, 2022

bevatsal1122 Aug 8, 2022

WikiRik left a comment

WikiRik Aug 13, 2022

bevatsal1122 Aug 13, 2022

WikiRik Aug 13, 2022

bevatsal1122 Aug 13, 2022

WikiRik Aug 13, 2022

WikiRik Aug 13, 2022

bevatsal1122 Aug 13, 2022

WikiRik Aug 13, 2022

bevatsal1122 Aug 13, 2022

WikiRik Aug 13, 2022

bevatsal1122 Aug 13, 2022

WikiRik Aug 13, 2022

bevatsal1122 Aug 13, 2022

bevatsal1122 commented Nov 14, 2022

pixelbucket-dev left a comment

pixelbucket-dev commented Nov 14, 2022

bevatsal1122 commented Nov 18, 2022

pixelbucket-dev commented Nov 21, 2022 •

edited

Loading

WikiRik commented Dec 2, 2022

pixelbucket-dev commented Jan 3, 2023

Added Exact Value/s Condition for String Length & Tests required with README.md Update #2019

Are you sure you want to change the base?

Added Exact Value/s Condition for String Length & Tests required with README.md Update #2019

Conversation

bevatsal1122 commented Aug 6, 2022 • edited Loading

Checklist

codecov bot commented Aug 6, 2022 • edited Loading

Codecov Report

bevatsal1122 commented Aug 6, 2022

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

Choose a reason for hiding this comment

WikiRik 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bevatsal1122 commented Nov 14, 2022

pixelbucket-dev left a comment

Choose a reason for hiding this comment

pixelbucket-dev commented Nov 14, 2022

bevatsal1122 commented Nov 18, 2022

pixelbucket-dev commented Nov 21, 2022 • edited Loading

WikiRik commented Dec 2, 2022

pixelbucket-dev commented Jan 3, 2023

bevatsal1122 commented Aug 6, 2022 •

edited

Loading

codecov bot commented Aug 6, 2022 •

edited

Loading

pixelbucket-dev commented Nov 21, 2022 •

edited

Loading