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

Build strict mode implementation #91

Merged

Conversation

VictorKunzler
Copy link
Contributor

Added a method to instantiate a structure in strict mode, if the instance is invalid, the method will throw an error

@VictorKunzler VictorKunzler force-pushed the buildStrictMode-implementation branch from 5c05eb9 to 4d4814a Compare July 1, 2019 21:04
Copy link
Owner

@talyssonoc talyssonoc left a comment

Choose a reason for hiding this comment

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

What happens if a nested structure is invalid, will it throw? If not, should it? If yes, we need tests for that.

return instance;
}

WrapperClass.buildStrictMode = buildStrictMode;
Copy link
Owner

Choose a reason for hiding this comment

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

I think we call it just buildStrict, what do you think? The mode word doesn't add too much value to the name.

@@ -139,6 +140,26 @@ describe('instantiating a structure', () => {
expect(user.name).to.equal('Not the default');
});
});

describe('instanding a structure with buildStrictMode', () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
describe('instanding a structure with buildStrictMode', () => {
describe('instantiating a structure with buildStrict', () => {

});

context('when object is valid', () => {
it('return a intance', () => {

This comment was marked as resolved.

@@ -0,0 +1,24 @@
# Build Strict

To instantiate a structure and automatically throw an error if that is invalid, you can use the buildStrict function.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
To instantiate a structure and automatically throw an error if that is invalid, you can use the buildStrict function.
To instantiate a structure that automatically throws an error if that is invalid, you can use the `buildStrict` method.

@@ -0,0 +1,24 @@
# Build Strict
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we should call this file strict-mode and change the title to Strict mode as well since it's the title of the item in the sidebar, what do you think?

Copy link
Contributor Author

@VictorKunzler VictorKunzler Jul 3, 2019

Choose a reason for hiding this comment

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

Change the file name or the folder name?

Copy link
Owner

Choose a reason for hiding this comment

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

the folder, the file has to be README 😄

age: Number
})(class User {});

var user = User.buildStrictMode({
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
var user = User.buildStrictMode({
var user = User.buildStrict({

it('throw an error', () => {
expect(() => {
User.buildStrict();
}).to.throw(Error, 'Invalid Attributes');
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can make this test more detailed using the with.property assertion as well, like this:

expect(badFn).to.throw(TypeError).with.property('code', 42);

This is not the actual code we'd use in the test, I took this example from here https://www.chaijs.com/api/bdd/#method_throw. We could use it to test the details as well. What do you think?

})
});

}).to.throw(Error, 'Invalid Attributes');
Copy link
Owner

Choose a reason for hiding this comment

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

Same thing here about using the with.property assertion.

docs/SUMMARY.md Outdated
@@ -16,10 +16,11 @@
* [Number validations](validation/number-validations.md)
* [Boolean validations](validation/boolean-validations.md)
* [Date validations](validation/date-validations.md)
* [Array validations](validation/array-validations.md)
* [Array validations](validation/array-validations.md)m
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo here

Suggested change
* [Array validations](validation/array-validations.md)m
* [Array validations](validation/array-validations.md)

function buildStrict(constructorArgs){
let instance = new WrapperClass(constructorArgs);

let {valid, errors} = instance.validate();
Copy link
Owner

Choose a reason for hiding this comment

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

You can use const here and on line 32 as well.

}];

expect(() => {

Copy link
Collaborator

Choose a reason for hiding this comment

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

✂️

kostadriano
kostadriano previously approved these changes Jul 8, 2019
talyssonoc
talyssonoc previously approved these changes Jul 8, 2019
@talyssonoc talyssonoc closed this Jul 8, 2019
@talyssonoc talyssonoc reopened this Jul 8, 2019
@talyssonoc talyssonoc merged commit f254c0d into talyssonoc:master Jul 8, 2019
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.

None yet

3 participants