solnic / virtus

[DISCONTINUED ] Attributes on Steroids for Plain Old Ruby Objects
MIT License
3.77k stars 229 forks source link

Strict mode can't be used with coercion disabled #251

Open valscion opened 10 years ago

valscion commented 10 years ago

When I use :strict => true mode together with :coerce => false, I would expect to receive errors when my input is not of the type defined with attribute method. What happens, though, is that my input is set as-is and no error is thrown.

Is this intended behavior, that strict: true has no effect when coercion is disabled? If it is, I believe documentation needs to be updated to reflect this.

The code I used to test this is:

class BlogPost
  include Virtus.model(strict: true, coerce: false)
  attribute :slug, String
end

Tested in console:

[1] test »  post = BlogPost.new(slug: {})
=> #<BlogPost:0x007f97d3d469e0 @slug={}>
[2] test »  post.slug
=> {}

Expected outcome was to get an error as strict mode was in use.

solnic commented 10 years ago

Currently strict mode is designed to work with coercions. I agree this should be documented though. I also started wondering if maybe I should change it so that it would raise also when coercion is turned off.

I'll think about this a bit more.

valscion commented 10 years ago

What about if an error would be thrown every time the given attribute was not of the type specified with attribute :name, <type> when coercion is disabled? An example:

class BlogPost
  include Virtus.model(strict: true, coerce: false)
  attribute :slug, String
end

# Error thrown:
BlogPost.new(slug: 2)
BlogPost.new(slug: {})

# No error
BlogPost.new(slug: "foo")

I think it makes sense, just making strict even stricter when it can't fall back to coercion.

mbj commented 10 years ago

@solnic One word: morpher :D

solnic commented 10 years ago

This is concerned with what we want to do. Not how. So morpher is kinda irrelevant here.

On Fri, Feb 28, 2014 at 3:18 PM, Markus Schirp notifications@github.com wrote:

@solnic One word: morpher :D

Reply to this email directly or view it on GitHub: https://github.com/solnic/virtus/issues/251#issuecomment-36353850

mbj commented 10 years ago

@solnic I just try to motivate myself into more OSS time, dont ruin this :D Sorry for the noise BTW.

solnic commented 10 years ago

This isn't noise no worries. It also motivates me to finally start working on integrating morpher. Otoh rom comes first so virtus has to wait.

On Fri, Feb 28, 2014 at 3:23 PM, Markus Schirp notifications@github.com wrote:

@solnic I just try to motivate myself into more OSS time, dont ruin this :D Sorry for the noise BTW.

Reply to this email directly or view it on GitHub: https://github.com/solnic/virtus/issues/251#issuecomment-36354589

mbj commented 10 years ago

I cant wait for wrocl{ove,lav}.