Inconsistent isValid() return value.

[yanickrochon]

I have stalled with #362. The current isValid() function may return a boolean or nothing at all, depending on the model's configured validation (if async or not). I see this as a relatively serious problem.

For example, let's say an application has grown to a respectable size, and is treating it's model according to their original design spec; some are async, other aren't. Some validations will be treated as 1) if (modelA.isValid()) { ... } or will be treated as 2) modelB.isValid(callback);.

Let's say, now, that modelA is requiring a new async validation, the application needs to be heavily refactored to adapt to the changes. Why aren't all validations treated as async in the first place, since it is not guaranteed that a model will be async or not? Why not just assuming that it always is to account the fact that it might just be async at some point, and therefore avoid unnecessary code maintenance?

I see this practice in many projects and I'm a strong believer that it is a very wrong practice.

[anatoliychakkaev]

Do not use it synchronously, always go for async style.

On 12 February 2014 17:08, Yanick Rochon notifications@github.com wrote:

I have stalled with #362 https://github.com/1602/jugglingdb/issues/362. The current isValid() function may return a boolean or nothing at all, depending on the model's configured validation (if async or not). I see this as a relatively serious problem.

For example, let's say an application has grown to a respectable size, and is treating it's model according to their original design spec; some are async, other aren't. Some validations will be treated as 1) if (modelA.isValid()) { ... } or will be treated as 2) modelB.isValid(callback);.

Let's say, now, that modelA is requiring a new async validation, the application needs to be heavily refactored to adapt to the changes. Why aren't all validations treated as async in the first place, since it is not guaranteed that a model will be async or not? Why not just assuming that it always is to account the fact that it might just be async at some point, and therefore avoid unnecessary code maintenance?

I see this practice in many projects and I'm a strong believer that it is a very wrong practice.

Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/378 .

[yanickrochon]

I'm asking because there are test cases like this one

var user = new User;
user.createdByAdmin = true;
user.isValid().should.be.false;
user.errors.pendingPeriod.should.eql(['can\'t be blank']);
user.pendingPeriod = 1
user.isValid().should.be.true;

and if I'm assuming async (like I expected it to be), then these use case will change and break current implementations. Are you saying that the next release will have breaking changes?

[anatoliychakkaev]

Yes this is the case, that's why I suggested to use async style even if you have only sync validations.

On 12 February 2014 17:16, Yanick Rochon notifications@github.com wrote:

I'm asking because there are test cases like this one

var user = new User; user.createdByAdmin = true; user.isValid().should.be.false; user.errors.pendingPeriod.should.eql(['can\'t be blank']); user.pendingPeriod = 1 user.isValid().should.be.true;

and if I'm assuming async (like I expected it to be), then these use case will change and break current implementations.

Reply to this email directly or view it on GitHubhttps://github.com/1602/jugglingdb/issues/378#issuecomment-34891789 .

Thanks, Anatoliy Chakkaev

[yanickrochon]

Alright-y!