#242 ConverterPrimitiveTypes calls 'new ConvertUtilsBean()' over and over

[Todd Gee]

The constructor of https://sourceforge.net/p/opencsv/source/ci/master/tree/src/main/java/com/opencsv/bean/ConverterPrimitiveTypes.java calls "new ConvertUtilsBean()" every time it's time to create an instance of this class.

This prevents anyone from using BeanUtils to register custom formats. (For users who don't have the ability to add formatting strings via annotations to the entity.) Also, this is very inefficient since this BeanUtils constructor is relatively heavyweight, deregistering and re-registering the entire set of converters every time.

Instead, call ConvertUtilsBean.getInstance() -- will speed things up and enable us to utilize the underlying BeanUtils functionality.

Or, if you want an isolated ConvertUtilBean instance for OpenCSV, create a static one, use that one over and over, and allow others to get a handle to it. Easy peasy.

thanks,
todd

[Scott Conway]

Will take a closer look at this. The main issue is that the getInstance in ConvertUtilBeans is protected so to use that we would have to make our own class that extends ConvertUtilBeans to expose the getInstance - like maybe a ConvertUtilBeansFactory.

[Scott Conway]

I found a quick and easy workaround when I looked in the commons code the ConvertUtilBean.getInstance calls BeanUtilsBean.getInstance().getConvertUtils() which was all public so I called that.

Unfortunately that made a lot of test fail so there will have to be some more work done on this. May go the static route instead.

Here are the failed tests

[ERROR] Failures:
[ERROR] AnnotationTest.testGoodDataByName:119->testGoodData:276 expected: <123202.202> but was: <123.202>
[ERROR] AnnotationTest.testGoodDataByPosition:214->testGoodData:276 expected: <123202.202> but was: <123.202>
[ERROR] AnnotationTest.testGoodDerivedDataByName:171->testGoodData:276 expected: <123202.202> but was: <123.202>
[ERROR] AnnotationTest.testGoodDerivedDataByPosition:229->testGoodData:276 expected: <123202.202> but was: <123.202>
[ERROR] AnnotationTest.testWriteFormatWithColumnPositions:1173 expected: <true> but was: <false>
[ERROR] AnnotationTest.testWriteFormatWithHeaderNames:1153 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeDerivedSubclass:443 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeDerivedSuperclass:464 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeDifferentOrderNameTypeFirst:970 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeDifferentOrderNameTypeLast:986 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeDifferentOrderPositionTypeFirst:922 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeDifferentOrderPositionTypeLast:938 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeMixedSingleMultipleBeans:280 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeMixedSingleMultipleBeansWithIterator:303 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeMultipleBeansOrdered:171 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeMultipleBeansOrderedFromStream:213 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeMultipleBeansOrderedWithIterator:192 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeMultipleBeansUnordered:235 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeMultipleBeansUnorderedWithIterator:258 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeNullOrderName:1002 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeNullOrderPosition:954 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeOptionalFieldsWithNull:328 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeOptionalNonContiguousField:347 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeSingleBeanNoQuotes:110 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeSingleOptionallyQuotedBean:132 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeSingleQuotedBean:147 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvTest.writeSpecifiedStrategy:371 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeDerivedSubclass:413 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeDerivedSuperclass:436 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeMixedSingleMultipleBeans:259 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeMultipleBeansOrdered:210 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeMultipleBeansUnordered:234 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeOptionalFieldsWithNull:287 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeOptionalNonContiguousField:309 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeSingleBeanNoQuotes:118 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeSingleOptionallyQuotedBean:134 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeSingleOptionallyQuotedBeanWithCSVParser:152 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeSingleOptionallyQuotedBeanWithPlainCSVWriter:169 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeSingleQuotedBean:185 expected: <true> but was: <false>
[ERROR] StatefulBeanToCsvWithCSVWriterTest.writeSpecifiedStrategy:331 expected: <true> but was: <false>
[INFO]
[ERROR] Tests run: 908, Failures: 40, Errors: 0, Skipped: 0
[INFO]</false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true></false></true>

And this was the code change in ConvertPrimitiveTypes

     public ConverterPrimitiveTypes(Class<?> type, String locale, String writeLocale, Locale errorLocale) {
        super(type, locale, writeLocale, errorLocale);
        if(this.locale == null) {
            readConverter = BeanUtilsBean.getInstance().getConvertUtils();
            readConverter.register(true, false, 0);
            readLocaleConverter = null;
        }
        else {
            readLocaleConverter = LocaleConvertUtilsBean.getInstance();
            readLocaleConverter.setDefaultLocale(this.locale);
            readConverter = null;
        }
        if(this.writeLocale == null) {
            writeConverter = BeanUtilsBean.getInstance().getConvertUtils();
            writeConverter.register(true, false, 0);
            writeLocaleConverter = null;
        }
        else {
            writeLocaleConverter = LocaleConvertUtilsBean.getInstance();
            writeLocaleConverter.setDefaultLocale(this.writeLocale);
            writeConverter = null;
        }
    }

[Scott Conway]

K - I realize my issue. I was trying to make both the converterutil and localconverterutil static and I cannot do that with the locale converter.

I made your original suggested change and all the tests are passing so I will do a few more tickets and then one more performance test and will then put it in the 5.7.2 or 5.8 release.

[Andrew Rucker Jones]

Has this change been released?

[Scott Conway]

yes - my apologies but aparently I did not update the wiki. I missed several of those in the 5.8 release for some reason. After ensuring that the fix is in by looking at the git history I did update the wiki.