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

Add symbol, undefined and null predicates #28

Merged
merged 2 commits into from
Nov 28, 2017
Merged

Conversation

SamVerschueren
Copy link
Collaborator

Completed all the primitive predicates by adding symbol, undefined and null.

@SamVerschueren SamVerschueren mentioned this pull request Nov 28, 2017
87 tasks
source/index.ts Outdated
@@ -49,6 +61,15 @@ Object.defineProperties(main, {
boolean: {
get: () => new BooleanPredicate()
},
symbol: {
get: () => new Predicate('symbol')
},
Copy link
Owner

Choose a reason for hiding this comment

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

Let's put symbol after null as it's not as common to check for it.

@SamVerschueren
Copy link
Collaborator Author

Fixed.

I'm also thinking in refactoring the Object.defineProperties block to something like this.

const predicateMap = new Map<string, () => Predicate>([
	['string', () => new StringPredicate()],
	['number', () => new NumberPredicate()],
	['boolean', () => new BooleanPredicate()],
	['array', () => new ArrayPredicate()],
	['date', () => new DatePredicate()]
]);

for (const propertyKey of Array.from(predicateMap.keys())) {
	Object.defineProperty(main, propertyKey, {
		get: predicateMap.get(propertyKey)
	});
}

It feels easier to maintain.

@sindresorhus
Copy link
Owner

sindresorhus commented Nov 28, 2017

Could also be:

const predicateMap = new Map<string, Predicate>([
	['string', StringPredicate],
	['number', NumberPredicate],
	['boolean', BooleanPredicate],
	['array', ArrayPredicate],
	['date', DatePredicate]
]);

for (const propertyKey of Array.from(predicateMap.keys())) {
	Object.defineProperty(main, propertyKey, {
		get: () => new predicateMap.get(propertyKey)
	});
}

@sindresorhus
Copy link
Owner

But I don't think it's worth it. Now it's just a tiny bit more boilerplate, but much clearer.

@sindresorhus sindresorhus merged commit 20d3ba7 into master Nov 28, 2017
@sindresorhus sindresorhus deleted the primitives branch November 28, 2017 13:48
@SamVerschueren
Copy link
Collaborator Author

But I don't think it's worth it. Now it's just a tiny bit more boilerplate, but much clearer.

True 👌

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.

2 participants