Refactor ActiveSupport::Inflector#ordinal #8117

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@vinnydiehl

This kills the repetitive "th".

Vinny Diehl
Refactor ActiveSupport::Inflector#ordinal
This kills the repetitive "th".
@@ -286,15 +286,10 @@ def safe_constantize(camel_cased_word)
# ordinal(-11) # => "th"
# ordinal(-1021) # => "st"
def ordinal(number)
- if (11..13).include?(number.to_i.abs % 100)
- "th"
+ if number.to_i.abs % 100 / 10 != 1 && (1..3).include?(last_digit = number.to_i.abs % 10)

This comment has been minimized.

Show comment Hide comment
@kytrinyx

kytrinyx Nov 4, 2012

Contributor

Would it be overkill to create a private method teen?(number)? to make it immediately obvious what that first bit is doing?

@kytrinyx

kytrinyx Nov 4, 2012

Contributor

Would it be overkill to create a private method teen?(number)? to make it immediately obvious what that first bit is doing?

@fxn

This comment has been minimized.

Show comment Hide comment
@fxn

fxn Nov 4, 2012

Owner

I think it is less readable.

Owner

fxn commented Nov 4, 2012

I think it is less readable.

@steveklabnik

This comment has been minimized.

Show comment Hide comment
@steveklabnik

steveklabnik Nov 4, 2012

Member

This is waaaay less readable. Sorry. Removing two "th"s to add that line of code is kinda crazy. Thanks for the patch, though.

Member

steveklabnik commented Nov 4, 2012

This is waaaay less readable. Sorry. Removing two "th"s to add that line of code is kinda crazy. Thanks for the patch, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment