-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add cardinality for numbers with two decimals #4
base: master
Are you sure you want to change the base?
Add cardinality for numbers with two decimals #4
Conversation
@henriquefreitas Muito obrigado pelo PR. É possível que você defina na descrição dele o intuito do mesmo? |
@henriquefreitas Muito obrigado. Agora, eu só estou pensativo sobre essa forma proposta por você de escrever a parte decimal por extenso. E nesse sentido eu gostaria que você lesse essa página que discorre sobre o assunto (e as formas de se escrever por extenso números decimais): https://professornews.com.br/index.php/component/content/article/181-dicas-de-redacao/5620-como-escrever-numeros-decimais-por-extenso |
@leandro de fato, a maneira como foi implementado, está mais para a forma popular, conforme indicado no link que você sugeriu. Para o caso de uso que estou utilizando a função, em específico, a forma popular é a mais adequada. Porém, concordo que pode não ser a mais útil a todos. Eu poderia contribuir implementando também a representação formal, deixando ao usuário a opção de escolher qual a forma que deseja utilizar (formal ou popular). Ou você acha que ficaria muito confuso? O que você sugere? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beleza! Deixei alguns comentários pertinentes ao longo do código que você enviou.
@@ -102,6 +124,20 @@ def cardinal_for_scale_of_thousands(number, scale, singular, plural) | |||
return high_order_units if remainder.zero? | |||
"#{high_order_units} e #{number_cardinal(remainder)}" | |||
end | |||
|
|||
def cardinal_for_fraction(fraction) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Esse método está sendo utilizado em algum lugar? 🤔
@@ -309,4 +309,20 @@ def test_negative_trillions | |||
def test_when_number_has_a_scale_bigger_than_trillion | |||
assert_raises(BrazilianCardinality::Number::NumberTooBigError) { @klass.number_cardinal(1_000_000_000_000_000) } | |||
end | |||
|
|||
def test_number_with_decimals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acho que seria interessante incluir pelo menos um teste que contemple mais de um zero após a vírgula.
O objetivo deste PR é estender a funcionalidade do método
Number#number_cardinal
, permitindo que seja retornada a descrição por extenso, de valores com até duas casas decimais, semelhante ao que acontece para o métodoCurrency#currency_cardinal
. O comportamento para números inteiros não foi alterado.Segue exemplo de utilização:
BrazilianCardinality::Number.number_cardinal(24.36) # returns 'vinte e quatro vírgula trinta e seis'