feat(size): allow to specify string length of random values #316

va-stefanek · 2022-11-13T11:31:12Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/ngneat/falso/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #299

stackblitz · 2022-11-13T11:31:14Z

Run & review this pull request in StackBlitz Codeflow.

NetanelBasal · 2022-11-17T08:11:59Z

README.md

+You can specify the length of elements you want to generate. Below is an example of generating 10 emails with length equal or smaller than 20 characters.
+
+```ts
+const emails = randEmail({ length: 10, size: 20 });


I think we should go we charSize to be more explicit. We need to check if we already use this name in other functions.

NetanelBasal · 2022-12-14T06:29:28Z

@va-stefanek any progress here?

PumpedSardines · 2022-12-17T14:40:31Z

I think the name should mention that the result can be less than size. isn't maxStringLength or maxCharCount better?

NetanelBasal · 2022-12-17T17:35:50Z

maxCharCount looks good to me

va-stefanek · 2022-12-26T20:59:19Z

Will work on it this week

NetanelBasal · 2022-12-29T06:41:44Z

README.md

+You can specify the length of elements you want to generate. Below is an example of generating 10 emails with length equal or smaller than 20 characters.
+
+```ts
+const emails = randEmail({ length: 10, maxCharCount: 20 });


The returned size can differ from the length value, right? I wonder if this is the desired result.

Isn't that length is just quantity of items and maxChartCount is length of that items?

But what happens when you request 20 items and only 10 passes the maxCharCount predicate?

Than there will be some duplicate values in return

Can you please add a test for this case?

https://github.com/ngneat/falso/pull/316/files#diff-b93ef17757be75c3c90f47b1c860c47f9eaa31865b28df81874fe2043686cfe5R12

There is such a test, should I increase the length and decrease maxCharCount?

Yes, please. In a new spec.

@NetanelBasal done

shaharkazaz · 2023-03-04T15:51:18Z

packages/falso/src/lib/core/core.ts

+    ? () =>
+        randElement(
+          data.filter((fakeData) =>
+            options?.maxCharCount && typeof fakeData === 'string'


@NetanelBasal Does it make sense that we have arrays with mixed data types? otherwise, I would test the array element and decide whether we need to filter.

We might want to add some more restrictions in the future, WDYT about creating an abstraction that applies these restrictions on the data set.

@NetanelBasal any update?

@NetanelBasal up

Sorry for the late response.

Does it make sense that we have arrays with mixed data types?

No

@shaharkazaz what is your suggestion based on the response from Netanel?

@shaharkazaz up

Given that we only have one data type per array we can move options?.maxCharCount && typeof fakeData === 'string' to only be tested once and not inside the filter loop.

I am not sure I understand it properly.

Do you suggest to code it like that

randElement( options?.maxCharCount && typeof data[0] === 'string' ? data.filter(fakeData => fakeData.length <= options.maxCharCount!) : data )

const dataSource = (data as FactoryFunction<T>); if (Array.isArray(data)) { let resolvedData = data; if (options?.maxCharCount && isString(data[0])) { resolvedData = data.filter((item) => item.length <= options.maxCharCount); } dataSource = () => randElement(resolvedData); }

@shaharkazaz done

shaharkazaz · 2023-07-06T10:42:46Z

packages/falso/src/lib/utils/isString.ts

+export const isString = (value: any) => {
+  return typeof value === 'string';
+};


Please rename the file to utils/validators.utils.

Suggested change

export const isString = (value: any) => {

return typeof value === 'string';

};

export function isString(value: any): value is string {

return typeof value === 'string';

}

shaharkazaz

Last ones 🙂

shaharkazaz · 2023-07-06T12:40:35Z

packages/falso/src/tests/max-char-count.spec.ts

+    result.forEach((movie) => expect(movie.length).toBeLessThanOrEqual(8));
+  });
+
+  it('should return 10 random movies with name length equal or smaller than 1', () => {


Suggested change

it('should return 10 random movies with name length equal or smaller than 1', () => {

it('should return 50 random movies with name length equal or smaller than 1', () => {

shaharkazaz · 2023-07-06T12:40:43Z

packages/falso/src/tests/max-char-count.spec.ts

+    result.forEach((movie) => expect(movie.length).toBeLessThanOrEqual(1));
+  });
+
+  it('should return 10 random movies with name length equal or smaller than 20', () => {


Suggested change

it('should return 10 random movies with name length equal or smaller than 20', () => {

it('should return 20 random movies with name length equal or smaller than 20', () => {

va-stefanek · 2023-08-19T13:14:19Z

@NetanelBasal up :)

va-stefanek force-pushed the feature/text-length branch 2 times, most recently from 3c41bd5 to 8243873 Compare November 13, 2022 11:34

NetanelBasal mentioned this pull request Nov 17, 2022

Control character length of individual random values #299

Closed

NetanelBasal reviewed Nov 17, 2022

View reviewed changes

va-stefanek force-pushed the feature/text-length branch from 8243873 to b2ae2a0 Compare December 28, 2022 10:19

va-stefanek requested a review from NetanelBasal December 28, 2022 10:20

NetanelBasal reviewed Dec 29, 2022

View reviewed changes

va-stefanek force-pushed the feature/text-length branch from b2ae2a0 to 89cf2a9 Compare March 3, 2023 12:01

va-stefanek requested a review from NetanelBasal March 3, 2023 12:01

NetanelBasal requested a review from shaharkazaz March 3, 2023 12:46

shaharkazaz reviewed Mar 4, 2023

View reviewed changes

va-stefanek force-pushed the feature/text-length branch from 89cf2a9 to d756f5b Compare July 6, 2023 09:09

shaharkazaz reviewed Jul 6, 2023

View reviewed changes

va-stefanek force-pushed the feature/text-length branch from d756f5b to 90ce4d0 Compare July 6, 2023 12:26

shaharkazaz reviewed Jul 6, 2023

View reviewed changes

va-stefanek force-pushed the feature/text-length branch from 90ce4d0 to aa72a6e Compare July 6, 2023 12:44

feat(size): allow to specify string length of random values

aa72a6e

shaharkazaz approved these changes Jul 6, 2023

View reviewed changes

shaharkazaz merged commit 4989449 into ngneat:main Aug 20, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(size): allow to specify string length of random values #316

feat(size): allow to specify string length of random values #316

va-stefanek commented Nov 13, 2022 •

edited

stackblitz bot commented Nov 13, 2022

NetanelBasal Nov 17, 2022

NetanelBasal commented Dec 14, 2022

PumpedSardines commented Dec 17, 2022

NetanelBasal commented Dec 17, 2022

va-stefanek commented Dec 26, 2022

NetanelBasal Dec 29, 2022

va-stefanek Dec 29, 2022

NetanelBasal Dec 29, 2022 •

edited

va-stefanek Dec 29, 2022 •

edited

NetanelBasal Dec 30, 2022

va-stefanek Dec 30, 2022

NetanelBasal Dec 30, 2022

va-stefanek Mar 3, 2023

shaharkazaz Mar 4, 2023

va-stefanek Apr 3, 2023

va-stefanek Jun 26, 2023

NetanelBasal Jun 27, 2023

va-stefanek Jun 28, 2023

va-stefanek Jul 4, 2023 •

edited

shaharkazaz Jul 5, 2023

va-stefanek Jul 5, 2023

shaharkazaz Jul 6, 2023

va-stefanek Jul 6, 2023

shaharkazaz Jul 6, 2023

shaharkazaz left a comment

shaharkazaz Jul 6, 2023

shaharkazaz Jul 6, 2023

va-stefanek commented Aug 19, 2023

	it('should return 10 random movies with name length equal or smaller than 1', () => {
	it('should return 50 random movies with name length equal or smaller than 1', () => {

	it('should return 10 random movies with name length equal or smaller than 20', () => {
	it('should return 20 random movies with name length equal or smaller than 20', () => {

feat(size): allow to specify string length of random values #316

feat(size): allow to specify string length of random values #316

Conversation

va-stefanek commented Nov 13, 2022 • edited

PR Checklist

PR Type

What is the current behavior?

stackblitz bot commented Nov 13, 2022

Choose a reason for hiding this comment

NetanelBasal commented Dec 14, 2022

PumpedSardines commented Dec 17, 2022

NetanelBasal commented Dec 17, 2022

va-stefanek commented Dec 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NetanelBasal Dec 29, 2022 • edited

Choose a reason for hiding this comment

va-stefanek Dec 29, 2022 • edited

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

va-stefanek Jul 4, 2023 • edited

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

shaharkazaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

va-stefanek commented Aug 19, 2023

va-stefanek commented Nov 13, 2022 •

edited

NetanelBasal Dec 29, 2022 •

edited

va-stefanek Dec 29, 2022 •

edited

va-stefanek Jul 4, 2023 •

edited