moiristo / deep_cloneable

This gem gives every ActiveRecord::Base object the possibility to do a deep clone that includes user specified associations.
MIT License
785 stars 89 forks source link

Unnecessary check for nil in except and only #101

Closed ristovskiv closed 5 years ago

ristovskiv commented 5 years ago

Once we check if options[:except] and if options[:only] the ternary operators for exceptions and for onlies are unnecessary since if they are nil they won't get inside the if condition block

moiristo commented 5 years ago

Ah indeed! Thanks!

ristovskiv commented 5 years ago

@moiristo I have a quick question, cause I might have ruined something with this PR regarding the Array(options[:except]) instead of [options[:except]].flatten. Did you support something like pirate.deep_clone include: :parrot, except: { parrot: [ :name ] } or you supported like this pirate.deep_clone include: :parrot, except: [ { parrot: [ :name ] } ]

moiristo commented 5 years ago

Hmm, yeah I think it's better to revert that part. It's quite old code and I personally haven't used the options myself. Should add a test for a case like below I think. The desired behavior should be like Array.wrap (Rails):

DEVELOPMENT [4] pry(main)> Array({ 1 => { 2 => [3,4] }})
=> [[1, {2=>[3, 4]}]]
DEVELOPMENT [5] pry(main)> Array.wrap({ 1 => { 2 => [3,4] }})
=> [{1=>{2=>[3, 4]}}]
DEVELOPMENT [6] pry(main)> [{ 1 => { 2 => [3,4] }}].flatten
=> [{1=>{2=>[3, 4]}}]