ppkarwasz commented on code in PR #367:
URL: https://github.com/apache/commons-beanutils/pull/367#discussion_r2269246088
##########
src/test/java/org/apache/commons/beanutils2/converters/BooleanConverterTest.java:
##########
@@ -44,6 +46,24 @@ void testAdditionalStrings() {
assertThrows(ConversionException.class, () ->
converter.convert(Boolean.class, "bogus"));
}
+ @Test
+ void testAdditionalStringsLocale() {
+ final String[] trueStrings = { "ναι" };
+ final String[] falseStrings = { "hayır" };
Review Comment:
```suggestion
final String[] trueStrings = { "\u03BD\u03B1\u03B9" }; // ναι
final String[] falseStrings = { "hay\u0131r" }; // hayır
```
Unicode letters in the code might be hard to debug, especially since the
upper case version of these strings looks exactly like ASCII.
##########
src/main/java/org/apache/commons/beanutils2/converters/BooleanConverter.java:
##########
@@ -136,20 +148,18 @@ public BooleanConverter(final String[] trueStrings, final
String[] falseStrings,
@Override
protected <T> T convertToType(final Class<T> type, final Object value)
throws Throwable {
if (Boolean.class.equals(type) || Boolean.TYPE.equals(type)) {
- // All the values in the trueStrings and falseStrings arrays are
- // guaranteed to be lower-case. By converting the input value
- // to lowercase too, we can use the efficient String.equals method
- // instead of the less-efficient String.equalsIgnoreCase method.
- final String stringValue = toLowerCase(value);
+ final String stringValue = toString(value);
+ Collator collator = Collator.getInstance(locale);
+ collator.setStrength(Collator.SECONDARY);
for (final String trueString : trueStrings) {
- if (trueString.equals(stringValue)) {
+ if (collator.compare(trueString, stringValue) == 0) {
Review Comment:
```suggestion
if (collator.equals(trueString, stringValue)) {
```
##########
src/test/java/org/apache/commons/beanutils2/converters/BooleanConverterTest.java:
##########
@@ -44,6 +46,24 @@ void testAdditionalStrings() {
assertThrows(ConversionException.class, () ->
converter.convert(Boolean.class, "bogus"));
}
+ @Test
+ void testAdditionalStringsLocale() {
+ final String[] trueStrings = { "ναι" };
+ final String[] falseStrings = { "hayır" };
+ final BooleanConverter converter = new BooleanConverter(trueStrings,
falseStrings);
+
+ converter.setLocale(Locale.forLanguageTag("el"));
+ for (final String trueValue : new String[] { "ναι", "Ναι" }) {
+ assertEquals(Boolean.TRUE, converter.convert(Boolean.class,
trueValue));
+ }
+
+ converter.setLocale(Locale.forLanguageTag("tr"));
+ for (final String falseValue : new String[] { "hayır", "Hayır" }) {
Review Comment:
```suggestion
// "hayır" and "HAYIR"
for (final String falseValue : new String[] { "hay\u0131r", "Hayır"
}) {
```
##########
src/main/java/org/apache/commons/beanutils2/converters/BooleanConverter.java:
##########
@@ -67,6 +56,11 @@ private static String[] copyStrings(final String[] src) {
*/
private String[] falseStrings = { "false", "no", "n", "off", "0" };
+ /**
+ * The locale to use for string comparisons.
+ */
+ private Locale locale = Locale.getDefault();
Review Comment:
I would either use `Locale.ROOT` as default value, since the default strings
are in English, or force the user to pass the `Locale` in the constructor.
As you remarked elsewhere, `commons-beanutils2` will be a breaking change
anyway, so we can modify the constructors and even introduce a builder pattern,
so we can add additional parameters in the future.
##########
src/test/java/org/apache/commons/beanutils2/converters/BooleanConverterTest.java:
##########
@@ -44,6 +46,24 @@ void testAdditionalStrings() {
assertThrows(ConversionException.class, () ->
converter.convert(Boolean.class, "bogus"));
}
+ @Test
+ void testAdditionalStringsLocale() {
+ final String[] trueStrings = { "ναι" };
+ final String[] falseStrings = { "hayır" };
+ final BooleanConverter converter = new BooleanConverter(trueStrings,
falseStrings);
+
+ converter.setLocale(Locale.forLanguageTag("el"));
+ for (final String trueValue : new String[] { "ναι", "Ναι" }) {
Review Comment:
```suggestion
// "ναι" and "ΝΑΙ"
for (final String trueValue : new String[] { "\u03BD\u03B1\u03B9",
"\u039D\u0391\u0399" }) {
```
##########
src/main/java/org/apache/commons/beanutils2/converters/BooleanConverter.java:
##########
@@ -136,20 +148,18 @@ public BooleanConverter(final String[] trueStrings, final
String[] falseStrings,
@Override
protected <T> T convertToType(final Class<T> type, final Object value)
throws Throwable {
if (Boolean.class.equals(type) || Boolean.TYPE.equals(type)) {
- // All the values in the trueStrings and falseStrings arrays are
- // guaranteed to be lower-case. By converting the input value
- // to lowercase too, we can use the efficient String.equals method
- // instead of the less-efficient String.equalsIgnoreCase method.
- final String stringValue = toLowerCase(value);
+ final String stringValue = toString(value);
+ Collator collator = Collator.getInstance(locale);
+ collator.setStrength(Collator.SECONDARY);
for (final String trueString : trueStrings) {
- if (trueString.equals(stringValue)) {
+ if (collator.compare(trueString, stringValue) == 0) {
return type.cast(Boolean.TRUE);
}
}
for (final String falseString : falseStrings) {
- if (falseString.equals(stringValue)) {
+ if (collator.compare(falseString, stringValue) == 0) {
Review Comment:
```suggestion
if (collator.equals(falseString, stringValue)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
