Skip to content

Commit

Permalink
Remove DEFAULT_SIZE from Numeric and Float
Browse files Browse the repository at this point in the history
  • Loading branch information
cored committed Jul 26, 2013
1 parent 50606c9 commit 9461a05
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 9 deletions.
2 changes: 0 additions & 2 deletions lib/axiom/attribute/float.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ class Attribute
# Represents a Float value in a relation tuple
class Float < Numeric

DEFAULT_SIZE = (-::Float::MAX..::Float::MAX).freeze

# The attribute type
#
# @return [Class<Types::Float>]
Expand Down
12 changes: 6 additions & 6 deletions lib/axiom/attribute/numeric.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ class Numeric < Object
Aggregate::Sum::Methods
include Equalizer.new(:name, :required?, :size)

DEFAULT_SIZE = (-::Float::INFINITY..::Float::INFINITY).freeze

inheritable_alias(range: :size)

# The attribute type
Expand Down Expand Up @@ -38,10 +36,12 @@ def self.type
# @api private
def initialize(_name, options = EMPTY_HASH)
super
size = options.fetch(:size, self.class::DEFAULT_SIZE).to_inclusive
@type = type.new do
minimum size.first
maximum size.last
if options.key?(:size)

This comment has been minimized.

Copy link
@dkubb

dkubb Jul 26, 2013

Owner

This is perfect. We want to minimize creating unnecessary types and reuse existing types as much as possible.

The only time we should be creating a new type is when we need to further constraint an existing one.

This comment has been minimized.

Copy link
@cored

cored via email Jul 26, 2013

Author Collaborator

This comment has been minimized.

Copy link
@dkubb

dkubb Jul 26, 2013

Owner

Yes, this is great. Exactly the direction we want to proceed. This makes it so we reuse the existing type in @type and lazily create a new type only if the user has specified further constraints.

It probably goes without saying, but in axiom-types, constraints are additive. You cannot loosen constraints on a new type, only add to them.

size = options.fetch(:size).to_inclusive

This comment has been minimized.

Copy link
@dkubb

dkubb Jul 26, 2013

Owner

At some point I wonder if we should refactor this so instead of :size it uses :minimum and :maximum options. With the size range it's harder to add a constraint on just one part. You have to specify the full range. Maybe you want the minimum to be 1, and you want to keep the maximum default.. but there's no way to specify that without duplicating it.

I only wish ruby allowed "open ended" Range objects (1.. or ..100) like a few other languages.

@type = type.new do
minimum size.first
maximum size.last
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/unit/axiom/attribute/float/size_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
context 'without :size option passed to constructor' do
let(:object) { described_class.new(:id) }

it { should eql(-Float::MAX..Float::MAX) }
it { should eql(Axiom::Types::NegativeInfinity..Axiom::Types::Infinity) }

This comment has been minimized.

Copy link
@dkubb

dkubb Jul 26, 2013

Owner

Actually, I wonder if maybe I should set the defaults in axiom-types to be this for Float? The defaults for Numeric still can be this, but since ruby actually does have a min and max limit for Float I should probably use that I think.

This comment has been minimized.

Copy link
@cored

cored via email Jul 26, 2013

Author Collaborator

This comment has been minimized.

Copy link
@dkubb

dkubb Jul 26, 2013

Owner

@cored ok, I've updated axiom-types to use Float::MIN..Float::MAX

I don't know what I was thinking here with using negative Float::MAX ;)

end

context 'with :size option passed to constructor' do
Expand Down

0 comments on commit 9461a05

Please sign in to comment.