-
Notifications
You must be signed in to change notification settings - Fork 93
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
Persian Language support #15
base: master
Are you sure you want to change the base?
Conversation
I'm sorry to write back so late, @omidp . I look forward merging your contribution. |
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.
Hello, thank you for your contribution. I recently took over the project and I hope your (well-aged) PR could still be merged! Before that could happen, some things need fixing (see comments), and you would have to add tests – this PR has none. I'll be happy to help if needed.
HundredsToWordsConverter hundredsToStringConverter = new HundredsToWordsConverter(persianValues.baseNumbers(), | ||
persianValues.twoDigitsNumberSeparator()); | ||
|
||
pl.allegro.finance.tradukisto.internal.languages.persian.IntegerToWordsConverter integerConverter = |
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.
please use import and not the full qualified class name
hundredsToStringConverter, persianValues.pluralForms()); | ||
|
||
return new Container(integerConverter, | ||
new pl.allegro.finance.tradukisto.internal.languages.persian.BigDecimalToBankingMoneyConverter(integerConverter, |
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.
please use import and not the full qualified class name
import static com.google.common.base.Preconditions.checkArgument; | ||
import static java.lang.String.format; | ||
|
||
public class BigDecimalToBankingMoneyConverter implements BigDecimalToStringConverter { |
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.
rename class to PersianBigDecimalToBankingMoneyConverter
|
||
Integer units = value.intValue(); | ||
Integer subunits = value.remainder(BigDecimal.ONE).multiply(new BigDecimal(100)).intValue(); | ||
if(subunits > 0) |
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.
add space after if
, add braces to if
and else
blocks
* @author omidp | ||
* | ||
*/ | ||
public class IntegerToWordsConverter extends pl.allegro.finance.tradukisto.internal.converters.IntegerToWordsConverter |
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.
rename class to PersianIntegerToWordsConverter
and add import for pl.allegro.finance.tradukisto.internal.converters.IntegerToWordsConverter
{ | ||
if(item != null && item.trim().length() > 0) | ||
{ | ||
if(counter > 1) |
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.
space after if
sb.append(string); | ||
} | ||
} | ||
else |
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.
add braces
@Override | ||
public Map<Integer, GenderForms> baseNumbers() | ||
{ | ||
return baseNumbersBuilder().put(0, "\u0635\u0641\u0631").put(1, "\u06CC\u06A9").put(2, "\u062F\u0648").put(3, "\u0633\u0647") |
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.
reformat so that each put
is on a separate line
@Override | ||
public List<PluralForms> pluralForms() | ||
{ | ||
return Arrays.asList(new PersianPluralForms(""), new PersianPluralForms("\u0647\u0632\u0627\u0631"), |
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.
reformat so that each constructor call is on a separate line
@Override | ||
public String currency() | ||
{ | ||
return ""; |
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.
add currency symbol or code
No description provided.