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

Cleanup core extensions #32

Closed
weppos opened this issue Sep 19, 2010 · 7 comments
Closed

Cleanup core extensions #32

weppos opened this issue Sep 19, 2010 · 7 comments

Comments

@weppos
Copy link
Member

weppos commented Sep 19, 2010

From #25

There's an small exception to my feedback. We can think about adding more .from_type methods with the purpose to simplify the core extensions provided in the core_extensions.rb file. In this case, we can create

# for the purpose of this example I separate Numeric
# into smaller classes
Money.from_fixnum(value, ...)
Money.from_float(value, ...)
Money.from_bigdecimal(value, ...)

class Fixnum
  def to_money(...)
    Money.from_fixnum(self, ...)
  end
end

class Float
  def to_money(...)
    Money.from_float(self, ...)
  end
end

class BigDecimal
  def to_money(...)
    Money.from_bigdecimal(self, ...)
  end
end

String deserves a special mention here because a String is a very data-rich money format. In fact, there are two types of string representations:

# with currency
"10 $"
"10 USD"
# without currency
"10"

This is the reason why we can try to provide two methods:

# A type-focused method, as for BigDecimal, Fixnum...
# which accepts only digit representations
# "1.0", "1000" and handle them as cents
Money.from_string

# A concept-focused method which implements a more advanced
# (and slow) detection mechanism
Money.parse(...)

The idea behind this change is to clean up the core_extensions by moving all the logic into the Money class.

@semmons99
Copy link
Member

I like the idea of separate methods for handling simple strings versus complex ones. We'll probably need to think about a way of deprecating the old String#to_money so that it works more inline with Money#from_string and a new method to String that is a synonym for Money#parse.

@weppos
Copy link
Member Author

weppos commented Sep 19, 2010

My idea, was to implement a simple guessing mechanism.
If the string contains something different than digits, use #parse. Otherwise, use #from_string.

@semmons99
Copy link
Member

so, something like a regex that detects anything other than digits, commas and periods?

@weppos
Copy link
Member Author

weppos commented Sep 20, 2010

Exactly.

@weppos
Copy link
Member Author

weppos commented Sep 22, 2010

I'm working on this.

@weppos
Copy link
Member Author

weppos commented Sep 22, 2010

Done. All tests passes.

I also took the time to cleanup the tests according to #28, increase documentation and test coverage.

http://github.com/RubyMoney/money/compare/issue_32

I didn't include the fix for #30 because I prefer to work on one ticket at time.

@weppos
Copy link
Member Author

weppos commented Sep 27, 2010

Cleanup core extensions (closed by 15460e9)

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants